| Message ID | 20210119181945.2071053-1-paullawrence@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | um: ubd: Fix crash from option parsing | expand |
Paul Lawrence <paullawrence@google.com> writes: > Below patch will cause NULL ptr dereferences if the optional filenames > are not present. > > Fixes: ef3ba87cb7c9 (um: ubd: Set device serial attribute from cmdline) > Signed-off-by: Paul Lawrence <paullawrence@google.com> Looks good. Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.com> Totally unrelated, but it seems the original patch writes a "(null)" string to the sysfs attribute, if no serial is defined. I think we should have a default serial UBDX in case the user didn't provide any. Thanks, > --- > arch/um/drivers/ubd_kern.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > index 13b1fe694b90..704989088f28 100644 > --- a/arch/um/drivers/ubd_kern.c > +++ b/arch/um/drivers/ubd_kern.c > @@ -371,15 +371,15 @@ static int ubd_setup_common(char *str, int *index_out, char **error_out) > > break_loop: > file = strsep(&str, ",:"); > - if (*file == '\0') > + if (file && *file == '\0') > file = NULL; > > backing_file = strsep(&str, ",:"); > - if (*backing_file == '\0') > + if (backing_file && *backing_file == '\0') > backing_file = NULL; > > serial = strsep(&str, ",:"); > - if (*serial == '\0') > + if (serial && *serial == '\0') > serial = NULL; > > if (backing_file && ubd_dev->no_cow) {
Hello, On Wed, 20 Jan 2021 03:19:45 +0900, Paul Lawrence wrote: > > Below patch will cause NULL ptr dereferences if the optional filenames > are not present. > > Fixes: ef3ba87cb7c9 (um: ubd: Set device serial attribute from cmdline) > Signed-off-by: Paul Lawrence <paullawrence@google.com> This was addressed/fixed by the below patch, though that one doesn't the first "file" variable check. http://lists.infradead.org/pipermail/linux-um/2020-December/000983.html There was another attempt to fix (with the same diff), btw. http://lists.infradead.org/pipermail/linux-um/2021-January/000998.html It seems that the patch is already queued but not upstreamed yet. -- Hajime
> This was addressed/fixed by the below patch, though that one doesn't > the first "file" variable check. I don't believe the first file variable can in fact be NULL, so my check was unnecessary. Pleased to see this has been fixed, we can abandon my patch now. Paul
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 13b1fe694b90..704989088f28 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -371,15 +371,15 @@ static int ubd_setup_common(char *str, int *index_out, char **error_out) break_loop: file = strsep(&str, ",:"); - if (*file == '\0') + if (file && *file == '\0') file = NULL; backing_file = strsep(&str, ",:"); - if (*backing_file == '\0') + if (backing_file && *backing_file == '\0') backing_file = NULL; serial = strsep(&str, ",:"); - if (*serial == '\0') + if (serial && *serial == '\0') serial = NULL; if (backing_file && ubd_dev->no_cow) {
Below patch will cause NULL ptr dereferences if the optional filenames are not present. Fixes: ef3ba87cb7c9 (um: ubd: Set device serial attribute from cmdline) Signed-off-by: Paul Lawrence <paullawrence@google.com> --- arch/um/drivers/ubd_kern.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)