libsvm 3.15 and 3.16 potential crash on svm_free_model_content()

[andre.dos.anjos@…]

The author of libsvm introduced the following lines of code on libsvm-3.15/svm.cpp (also true for libsvm-3.16), for model destructors - svm_free_model_content(), around line 2977:

  free(model_ptr->sv_indices);
  model_ptr->sv_indices = NULL;

This works well when svm_train() is called before svm_free_model_content() because svm_train() initializes model_ptr->sv_indices properly. This is not the case for svm_load_model(), that loads a pre-trained model. The variable sv_indices is never initialized (to NULL) so, depending on code layout and compilation options, its value may be set to != NULL. In those cases, the newly introduced code will cause a free() error (trying to deallocate unallocated memory) which leads to program abortion.

To fix this problem, it suffices to initialize sv_indices properly. Patch attached. Could you please add this patch and re-state the build?

Thanks, A

[andre.dos.anjos@…]

Patch to fix model_ptr->sv_indices initialization issue

[ryandesign (Ryan Carsten Schmidt)]

Could you please refer us to the upstream bug report about this problem?

[andre.dos.anjos@…]

There is no "upstream" for this package. The owner does not maintain such a thing. I've e-mailed him with the same fix and I have verified it works on our local build server.

As it is, the library produced by this package cannot be used reliably in other applications. This is crashing our test units on the port "bob":

​https://www.idiap.ch/software/bob/buildbot/builders/macosx-10.8-x86_64-debug/builds/111/steps/nosetest/logs/stdio

[ryandesign (Ryan Carsten Schmidt)]

Replying to andre.dos.anjos@…:

There is no "upstream" for this package. The owner does not maintain such a thing. I've e-mailed him with the same fix and I have verified it works on our local build server.

Not to argue, but there do appear to be active upstream developers. There is a ​homepage, with names of developers listed, and the latest version of libsvm was released four days ago.

Point is, I'm not a C programmer and I don't know libsvm, so I can't evaluate the correctness of your proposed patch. I'm not saying we won't apply this patch, just that when we do apply patches we would prefer that they have been at least acknowledged by the developers, ideally accepted and committed to their source repository. If there is no public issue tracker or discussion about this issue to which you can provide a link, and no public source repository which we can peruse, then please let us know of the developers' email response.

[andre.dos.anjos@…]

So, here you have the conversation. Can you consider the patch now?

Delivered-To: andre.dos.anjos@...
Received: by 10.59.7.130 with SMTP id dc2csp70684ved;
        Thu, 31 Jan 2013 04:48:46 -0800 (PST)
...
From: Chih-Jen Lin <cjlin@...>
...
Date: Thu, 31 Jan 2013 20:48:37 +0800
To: =?iso-8859-1?Q?Andr=E9?= Anjos <andre.dos.anjos@...>
Subject: libsvm-3.15 & 3.16 initialization bug
X-ASG-Orig-Subj: libsvm-3.15 & 3.16 initialization bug


Yes, you are right. We will fix this in the next
release.

Best,
Chih-Jen


André Anjos writes:
 > Hello,
 >
 > Please note that the newly introduced free() call at
 > svm_free_model_content() (lines 2976+1):
 >
 >   free(model_ptr->sv_indices);
 >   model_ptr->sv_indices = NULL;
 >
 > Potentially triggers a crash because "sv_indices" is
 > never properly initialized when using svm_load_model
 > (). It is only done if svm_train() is called.
 >
 > This means that any "svm_load_model()" usage,
 > depending on compile options and other code layout
 > details, may load a model with an uninitialized
 > value of "sv_indices" (!= 0) and the code indicated
 > above would crash in this case.
 >
 > I suggest the following patch @ svm_load_model()
 > (arond line 2749):
 >
 > model->sv_indices = NULL;
 >
 > This should do the trick.
 >
 > Best,

[humem (humem)]

Thank you for your reporting a potential problem and providing the patch. Committed in r102369.