* [PATCH] um: Fix null pointer dereference when parsing ubd commandline arguments
@ 2021-01-02 14:03 Joshua Hawking
2021-01-02 14:08 ` Christopher Obbard
0 siblings, 1 reply; 3+ messages in thread
From: Joshua Hawking @ 2021-01-02 14:03 UTC (permalink / raw)
To: linux-um
From: Adam Watson (aw414141@gmail.com)
When passing one or two arguments to ubd during UML setup - i.e.
ubd0=File or ubd0=File,Backing_File - the parsing code introduced in commit
ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline") does
not check
whether strsep consumed the entire string without finding a delimeter
last time
it was called (and so has set str to NULL, causing the next output of strsep
on that string to be NULL) before attempting to dereference the output of it
inside the if statements. For example, with two arguments (and only 1
comma/colon), serial will be NULL, and (*serial == '\0') causes a null
pointer
dereference.
Signed-off-by: Adam Watson (aw414141@gmail.com)
Signed-off-by: Joshua Hawking (joshuahawking1@gmail.com)
Tested-by: Joshua Hawking (joshuahawking1@gmail.com)
Fixes: ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline")
---
--- b/arch/um/drivers/ubd_kern.c 2021-01-02 13:13:55.995018942 +0000
+++ a/arch/um/drivers/ubd_kern.c 2021-01-02 13:16:16.847023905 +0000
@@ -375,11 +375,11 @@ break_loop:
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) {
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] um: Fix null pointer dereference when parsing ubd commandline arguments
2021-01-02 14:03 [PATCH] um: Fix null pointer dereference when parsing ubd commandline arguments Joshua Hawking
@ 2021-01-02 14:08 ` Christopher Obbard
2021-01-02 14:11 ` Joshua Hawking
0 siblings, 1 reply; 3+ messages in thread
From: Christopher Obbard @ 2021-01-02 14:08 UTC (permalink / raw)
To: Joshua Hawking; +Cc: linux-um
Hi Joshua,
See http://lists.infradead.org/pipermail/linux-um/2020-December/000983.html
this patch hasn't yet been applied to linux-next, it will probably
arrive for 5.11rc2
Thanks!
Chris
On Sat, 2 Jan 2021 at 14:04, Joshua Hawking <joshuahawking1@gmail.com> wrote:
>
> From: Adam Watson (aw414141@gmail.com)
>
> When passing one or two arguments to ubd during UML setup - i.e.
> ubd0=File or ubd0=File,Backing_File - the parsing code introduced in commit
> ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline") does
> not check
> whether strsep consumed the entire string without finding a delimeter
> last time
> it was called (and so has set str to NULL, causing the next output of strsep
> on that string to be NULL) before attempting to dereference the output of it
> inside the if statements. For example, with two arguments (and only 1
> comma/colon), serial will be NULL, and (*serial == '\0') causes a null
> pointer
> dereference.
>
> Signed-off-by: Adam Watson (aw414141@gmail.com)
> Signed-off-by: Joshua Hawking (joshuahawking1@gmail.com)
> Tested-by: Joshua Hawking (joshuahawking1@gmail.com)
> Fixes: ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline")
> ---
> --- b/arch/um/drivers/ubd_kern.c 2021-01-02 13:13:55.995018942 +0000
> +++ a/arch/um/drivers/ubd_kern.c 2021-01-02 13:16:16.847023905 +0000
> @@ -375,11 +375,11 @@ break_loop:
> 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) {
>
>
>
>
> _______________________________________________
> linux-um mailing list
> linux-um@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] um: Fix null pointer dereference when parsing ubd commandline arguments
2021-01-02 14:08 ` Christopher Obbard
@ 2021-01-02 14:11 ` Joshua Hawking
0 siblings, 0 replies; 3+ messages in thread
From: Joshua Hawking @ 2021-01-02 14:11 UTC (permalink / raw)
To: Christopher Obbard; +Cc: linux-um
Aha we didn't see this patch, good to see it was already fixed, thanks!
On 02/01/2021 14:08, Christopher Obbard wrote:
> Hi Joshua,
> See http://lists.infradead.org/pipermail/linux-um/2020-December/000983.html
> this patch hasn't yet been applied to linux-next, it will probably
> arrive for 5.11rc2
>
> Thanks!
> Chris
>
> On Sat, 2 Jan 2021 at 14:04, Joshua Hawking <joshuahawking1@gmail.com> wrote:
>> From: Adam Watson (aw414141@gmail.com)
>>
>> When passing one or two arguments to ubd during UML setup - i.e.
>> ubd0=File or ubd0=File,Backing_File - the parsing code introduced in commit
>> ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline") does
>> not check
>> whether strsep consumed the entire string without finding a delimeter
>> last time
>> it was called (and so has set str to NULL, causing the next output of strsep
>> on that string to be NULL) before attempting to dereference the output of it
>> inside the if statements. For example, with two arguments (and only 1
>> comma/colon), serial will be NULL, and (*serial == '\0') causes a null
>> pointer
>> dereference.
>>
>> Signed-off-by: Adam Watson (aw414141@gmail.com)
>> Signed-off-by: Joshua Hawking (joshuahawking1@gmail.com)
>> Tested-by: Joshua Hawking (joshuahawking1@gmail.com)
>> Fixes: ef3ba87cb7c9 ("um: ubd: Set device serial attribute from cmdline")
>> ---
>> --- b/arch/um/drivers/ubd_kern.c 2021-01-02 13:13:55.995018942 +0000
>> +++ a/arch/um/drivers/ubd_kern.c 2021-01-02 13:16:16.847023905 +0000
>> @@ -375,11 +375,11 @@ break_loop:
>> 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) {
>>
>>
>>
>>
>> _______________________________________________
>> linux-um mailing list
>> linux-um@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-um
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-02 14:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-02 14:03 [PATCH] um: Fix null pointer dereference when parsing ubd commandline arguments Joshua Hawking
2021-01-02 14:08 ` Christopher Obbard
2021-01-02 14:11 ` Joshua Hawking
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox