Richard W.M. Jones
2023-Jun-20 08:08 UTC
[Libguestfs] [PATCH v3] ldmtool: fix NULL pointer dereference
On Tue, Jun 20, 2023 at 05:00:24PM +0900, Vincent Mailhol wrote:> If /sys/block can not be opened, get_devices() returns NULL. > > cmdline() does not check this result and below code snippet: > > scanned = get_devices(); > devices = (gchar **) scanned->data; > > results in a segmentation fault. > > Add a check on scanned. > > Relevant logs: > > Unable to open /sys/block: No such file or directory > [ 0.777352] ldmtool[164]: segfault at 0 ip 0000563a225cd6a5 sp 00007ffe54965a60 error 4 in ldmtool[563a225cb000+3000] > [ 0.778278] Code: 18 64 48 33 1c 25 28 00 00 00 75 5e 48 83 c4 28 5b 5d 41 5c 41 5d 41 5e 41 5f c3 66 2e 0f 1f 84 00 00 00 00 00 e8 db fd ff ff <4c> 8b 20 48 89 44 24 08 4c 89 e7 e8 0b e1 ff ff 45 31 c0 4c 89 e1 > > Fixes: 25d9635e4ee5 ("Add ldmtool") > Signed-off-by: Vincent Mailhol <mailhol.vincent at wanadoo.fr> > --- > > * Changelog * > > v2 -> v3 > > * Fix the From: tag (incorrect e-mail address, sorry for the noise). > > v1 -> v2 > > * Directly return FALSE instead of goto error. Jumping to the error > label bypasses jb's declaration thus resulting in an undefined > behavior. > > --- > src/ldmtool.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/ldmtool.c b/src/ldmtool.c > index 6957c1a..dbe2c8c 100644 > --- a/src/ldmtool.c > +++ b/src/ldmtool.c > @@ -746,6 +746,8 @@ cmdline(LDM * const ldm, gchar **devices, > GArray * scanned = NULL; > if (!devices) { > scanned = get_devices(); > + if (!scanned) > + return FALSE; > devices = (gchar **) scanned->data; > }Seems fine, based on Laszlo's analysis of the first version, thus: Acked-by: Richard W.M. Jones <rjones at redhat.com> I believe I will be able to push this patch (or if not, I'll ask Matt to do it later). Is this version OK Laszlo? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Laszlo Ersek
2023-Jun-20 09:15 UTC
[Libguestfs] [PATCH v3] ldmtool: fix NULL pointer dereference
On 6/20/23 10:08, Richard W.M. Jones wrote:> On Tue, Jun 20, 2023 at 05:00:24PM +0900, Vincent Mailhol wrote: >> If /sys/block can not be opened, get_devices() returns NULL. >> >> cmdline() does not check this result and below code snippet: >> >> scanned = get_devices(); >> devices = (gchar **) scanned->data; >> >> results in a segmentation fault. >> >> Add a check on scanned. >> >> Relevant logs: >> >> Unable to open /sys/block: No such file or directory >> [ 0.777352] ldmtool[164]: segfault at 0 ip 0000563a225cd6a5 sp 00007ffe54965a60 error 4 in ldmtool[563a225cb000+3000] >> [ 0.778278] Code: 18 64 48 33 1c 25 28 00 00 00 75 5e 48 83 c4 28 5b 5d 41 5c 41 5d 41 5e 41 5f c3 66 2e 0f 1f 84 00 00 00 00 00 e8 db fd ff ff <4c> 8b 20 48 89 44 24 08 4c 89 e7 e8 0b e1 ff ff 45 31 c0 4c 89 e1 >> >> Fixes: 25d9635e4ee5 ("Add ldmtool") >> Signed-off-by: Vincent Mailhol <mailhol.vincent at wanadoo.fr> >> --- >> >> * Changelog * >> >> v2 -> v3 >> >> * Fix the From: tag (incorrect e-mail address, sorry for the noise). >> >> v1 -> v2 >> >> * Directly return FALSE instead of goto error. Jumping to the error >> label bypasses jb's declaration thus resulting in an undefined >> behavior. >> >> --- >> src/ldmtool.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/src/ldmtool.c b/src/ldmtool.c >> index 6957c1a..dbe2c8c 100644 >> --- a/src/ldmtool.c >> +++ b/src/ldmtool.c >> @@ -746,6 +746,8 @@ cmdline(LDM * const ldm, gchar **devices, >> GArray * scanned = NULL; >> if (!devices) { >> scanned = get_devices(); >> + if (!scanned) >> + return FALSE; >> devices = (gchar **) scanned->data; >> } > > Seems fine, based on Laszlo's analysis of the first version, thus: > > Acked-by: Richard W.M. Jones <rjones at redhat.com> > > I believe I will be able to push this patch (or if not, I'll ask Matt > to do it later). Is this version OK Laszlo?Reviewed-by: Laszlo Ersek <lersek at redhat.com> Thanks! Laszlo> > Rich. >