* [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-26 12:07 ` Mikulas Patocka
0 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-26 12:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > If we pass too short string to "hex2bin" (and the string size without the
> > terminating NUL character is even), "hex2bin" reads one byte after the
> > terminating NUL character. This patch fixes it.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > Cc: stable@vger.kernel.org
>
> You need to provide a Fixes tag.
OK. Here I resend it with the "Fixes" tag.
> And on top of that it would be nice to understand if we need to
> support half-bytes, but in any case it's not a scope of the patch
> right now.
Do you think that there are any users who need this?
> --
> With Best Regards,
> Andy Shevchenko
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
If we pass too short string to "hex2bin" (and the string size without the
terminating NUL character is even), "hex2bin" reads one byte after the
terminating NUL character. This patch fixes it.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: b78049831ffe ("lib: add error checking to hex2bin")
Cc: stable@vger.kernel.org
---
lib/hexdump.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Index: linux-2.6/lib/hexdump.c
===================================================================
--- linux-2.6.orig/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
+++ linux-2.6/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
@@ -45,10 +45,13 @@ EXPORT_SYMBOL(hex_to_bin);
int hex2bin(u8 *dst, const char *src, size_t count)
{
while (count--) {
- int hi = hex_to_bin(*src++);
- int lo = hex_to_bin(*src++);
+ int hi, lo;
- if ((hi < 0) || (lo < 0))
+ hi = hex_to_bin(*src++);
+ if (hi < 0)
+ return -EINVAL;
+ lo = hex_to_bin(*src++);
+ if (lo < 0)
return -EINVAL;
*dst++ = (hi << 4) | lo;
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-26 12:07 ` [dm-devel] " Mikulas Patocka
@ 2022-04-26 13:18 ` Andy Shevchenko
-1 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-26 13:18 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > >
> > > If we pass too short string to "hex2bin" (and the string size without the
> > > terminating NUL character is even), "hex2bin" reads one byte after the
> > > terminating NUL character. This patch fixes it.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > > Cc: stable@vger.kernel.org
> >
> > You need to provide a Fixes tag.
>
> OK. Here I resend it with the "Fixes" tag.
>
> > And on top of that it would be nice to understand if we need to
> > support half-bytes, but in any case it's not a scope of the patch
> > right now.
>
> Do you think that there are any users who need this?
If my memory doesn't do any tricks with me, I have seen the patterns like
hex2bin() + hex_to_bin() in some places in the kernel.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-26 13:18 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-26 13:18 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > >
> > > If we pass too short string to "hex2bin" (and the string size without the
> > > terminating NUL character is even), "hex2bin" reads one byte after the
> > > terminating NUL character. This patch fixes it.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > > Cc: stable@vger.kernel.org
> >
> > You need to provide a Fixes tag.
>
> OK. Here I resend it with the "Fixes" tag.
>
> > And on top of that it would be nice to understand if we need to
> > support half-bytes, but in any case it's not a scope of the patch
> > right now.
>
> Do you think that there are any users who need this?
If my memory doesn't do any tricks with me, I have seen the patterns like
hex2bin() + hex_to_bin() in some places in the kernel.
--
With Best Regards,
Andy Shevchenko
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-26 12:07 ` [dm-devel] " Mikulas Patocka
@ 2022-04-26 13:19 ` Andy Shevchenko
-1 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-26 13:19 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
...
> > You need to provide a Fixes tag.
>
> OK. Here I resend it with the "Fixes" tag.
Still shadows error codes.
> + return -EINVAL;
> return -EINVAL;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-26 13:19 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-26 13:19 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
...
> > You need to provide a Fixes tag.
>
> OK. Here I resend it with the "Fixes" tag.
Still shadows error codes.
> + return -EINVAL;
> return -EINVAL;
--
With Best Regards,
Andy Shevchenko
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-26 13:19 ` [dm-devel] " Andy Shevchenko
@ 2022-04-26 15:29 ` Mikulas Patocka
-1 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-26 15:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> ...
>
> > > You need to provide a Fixes tag.
> >
> > OK. Here I resend it with the "Fixes" tag.
>
> Still shadows error codes.
>
> > + return -EINVAL;
>
> > return -EINVAL;
What do you mean? What's wrong with "return -EINVAL"?
Mikulas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-26 15:29 ` Mikulas Patocka
0 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-26 15:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> ...
>
> > > You need to provide a Fixes tag.
> >
> > OK. Here I resend it with the "Fixes" tag.
>
> Still shadows error codes.
>
> > + return -EINVAL;
>
> > return -EINVAL;
What do you mean? What's wrong with "return -EINVAL"?
Mikulas
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-26 15:29 ` [dm-devel] " Mikulas Patocka
@ 2022-04-27 12:24 ` Andy Shevchenko
-1 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-27 12:24 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
...
> > Still shadows error codes.
> >
> > > + return -EINVAL;
> >
> > > return -EINVAL;
>
> What do you mean? What's wrong with "return -EINVAL"?
The actual error code is returned by hex_to_bin(). What is the point
of shadowing it with the explicit value?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-27 12:24 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-27 12:24 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
...
> > Still shadows error codes.
> >
> > > + return -EINVAL;
> >
> > > return -EINVAL;
>
> What do you mean? What's wrong with "return -EINVAL"?
The actual error code is returned by hex_to_bin(). What is the point
of shadowing it with the explicit value?
--
With Best Regards,
Andy Shevchenko
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-27 12:24 ` [dm-devel] " Andy Shevchenko
@ 2022-04-27 14:10 ` Mikulas Patocka
-1 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-27 14:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Wed, 27 Apr 2022, Andy Shevchenko wrote:
> On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> ...
>
> > > Still shadows error codes.
> > >
> > > > + return -EINVAL;
> > >
> > > > return -EINVAL;
> >
> > What do you mean? What's wrong with "return -EINVAL"?
>
> The actual error code is returned by hex_to_bin(). What is the point
> of shadowing it with the explicit value?
hex_to_bin returns -1 on error, hex2bin returns -EINVAL on error.
This is inconsistent and it may be fixed (after verifying all the
hex_to_bin callers - a quick grep over the source shows that there is "if
((k = hex_to_bin(in_str[j--])) != -1)").
But for the purpose of fixing this bug, we should preserve the behavior
and return -1 and -EINVAL just like it was before.
Mikulas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-27 14:10 ` Mikulas Patocka
0 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-27 14:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Wed, 27 Apr 2022, Andy Shevchenko wrote:
> On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> ...
>
> > > Still shadows error codes.
> > >
> > > > + return -EINVAL;
> > >
> > > > return -EINVAL;
> >
> > What do you mean? What's wrong with "return -EINVAL"?
>
> The actual error code is returned by hex_to_bin(). What is the point
> of shadowing it with the explicit value?
hex_to_bin returns -1 on error, hex2bin returns -EINVAL on error.
This is inconsistent and it may be fixed (after verifying all the
hex_to_bin callers - a quick grep over the source shows that there is "if
((k = hex_to_bin(in_str[j--])) != -1)").
But for the purpose of fixing this bug, we should preserve the behavior
and return -1 and -EINVAL just like it was before.
Mikulas
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] hex2bin: fix access beyond string end
2022-04-27 14:10 ` [dm-devel] " Mikulas Patocka
@ 2022-04-27 14:49 ` Andy Shevchenko
-1 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-27 14:49 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Linus Torvalds, Andy Shevchenko, Mimi Zohar,
device-mapper development, Linux Kernel Mailing List,
Mike Snitzer, Milan Broz
On Wed, Apr 27, 2022 at 4:10 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Wed, 27 Apr 2022, Andy Shevchenko wrote:
> > On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > ...
> >
> > > > Still shadows error codes.
> > > >
> > > > > + return -EINVAL;
> > > >
> > > > > return -EINVAL;
> > >
> > > What do you mean? What's wrong with "return -EINVAL"?
> >
> > The actual error code is returned by hex_to_bin(). What is the point
> > of shadowing it with the explicit value?
>
> hex_to_bin returns -1 on error, hex2bin returns -EINVAL on error.
>
> This is inconsistent and it may be fixed (after verifying all the
> hex_to_bin callers - a quick grep over the source shows that there is "if
> ((k = hex_to_bin(in_str[j--])) != -1)").
>
> But for the purpose of fixing this bug, we should preserve the behavior
> and return -1 and -EINVAL just like it was before.
This is clear now to me. So, by improving a commit message you may
make it clear to everybody who reads your change.
With the updated commit message,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
@ 2022-04-27 14:49 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2022-04-27 14:49 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Andy Shevchenko, Mike Snitzer, Linux Kernel Mailing List,
Mimi Zohar, device-mapper development, Linus Torvalds,
Milan Broz
On Wed, Apr 27, 2022 at 4:10 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Wed, 27 Apr 2022, Andy Shevchenko wrote:
> > On Tue, Apr 26, 2022 at 5:29 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > On Tue, Apr 26, 2022 at 08:07:44AM -0400, Mikulas Patocka wrote:
> > > > > On Tue, 26 Apr 2022, Andy Shevchenko wrote:
> > > > > > On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > ...
> >
> > > > Still shadows error codes.
> > > >
> > > > > + return -EINVAL;
> > > >
> > > > > return -EINVAL;
> > >
> > > What do you mean? What's wrong with "return -EINVAL"?
> >
> > The actual error code is returned by hex_to_bin(). What is the point
> > of shadowing it with the explicit value?
>
> hex_to_bin returns -1 on error, hex2bin returns -EINVAL on error.
>
> This is inconsistent and it may be fixed (after verifying all the
> hex_to_bin callers - a quick grep over the source shows that there is "if
> ((k = hex_to_bin(in_str[j--])) != -1)").
>
> But for the purpose of fixing this bug, we should preserve the behavior
> and return -1 and -EINVAL just like it was before.
This is clear now to me. So, by improving a commit message you may
make it clear to everybody who reads your change.
With the updated commit message,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] hex2bin: fix access beyond string end
2022-04-27 14:49 ` [dm-devel] " Andy Shevchenko
@ 2022-04-27 15:26 ` Mikulas Patocka
-1 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-27 15:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Shevchenko, Mimi Zohar, device-mapper development,
Linux Kernel Mailing List, Mike Snitzer, Milan Broz
If we pass too short string to "hex2bin" (and the string size without the
terminating NUL character is even), "hex2bin" reads one byte after the
terminating NUL character. This patch fixes it.
Note that hex_to_bin returns -1 on error and hex2bin return -EINVAL on
error - so we can't just return the variable "hi" or "lo" on error. This
inconsistency may be fixed in the next merge window, but for the purpose
of fixing this bug, we just preserve the existing behavior and return -1
and -EINVAL.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Fixes: b78049831ffe ("lib: add error checking to hex2bin")
Cc: stable@vger.kernel.org
---
lib/hexdump.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Index: linux-2.6/lib/hexdump.c
===================================================================
--- linux-2.6.orig/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
+++ linux-2.6/lib/hexdump.c 2022-04-27 17:16:38.000000000 +0200
@@ -45,10 +45,13 @@ EXPORT_SYMBOL(hex_to_bin);
int hex2bin(u8 *dst, const char *src, size_t count)
{
while (count--) {
- int hi = hex_to_bin(*src++);
- int lo = hex_to_bin(*src++);
+ int hi, lo;
- if ((hi < 0) || (lo < 0))
+ hi = hex_to_bin(*src++);
+ if (unlikely(hi < 0))
+ return -EINVAL;
+ lo = hex_to_bin(*src++);
+ if (unlikely(lo < 0))
return -EINVAL;
*dst++ = (hi << 4) | lo;
^ permalink raw reply [flat|nested] 20+ messages in thread* [dm-devel] [PATCH v3] hex2bin: fix access beyond string end
@ 2022-04-27 15:26 ` Mikulas Patocka
0 siblings, 0 replies; 20+ messages in thread
From: Mikulas Patocka @ 2022-04-27 15:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Mike Snitzer, device-mapper development, Mimi Zohar,
Linux Kernel Mailing List, Andy Shevchenko, Milan Broz
If we pass too short string to "hex2bin" (and the string size without the
terminating NUL character is even), "hex2bin" reads one byte after the
terminating NUL character. This patch fixes it.
Note that hex_to_bin returns -1 on error and hex2bin return -EINVAL on
error - so we can't just return the variable "hi" or "lo" on error. This
inconsistency may be fixed in the next merge window, but for the purpose
of fixing this bug, we just preserve the existing behavior and return -1
and -EINVAL.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Fixes: b78049831ffe ("lib: add error checking to hex2bin")
Cc: stable@vger.kernel.org
---
lib/hexdump.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Index: linux-2.6/lib/hexdump.c
===================================================================
--- linux-2.6.orig/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
+++ linux-2.6/lib/hexdump.c 2022-04-27 17:16:38.000000000 +0200
@@ -45,10 +45,13 @@ EXPORT_SYMBOL(hex_to_bin);
int hex2bin(u8 *dst, const char *src, size_t count)
{
while (count--) {
- int hi = hex_to_bin(*src++);
- int lo = hex_to_bin(*src++);
+ int hi, lo;
- if ((hi < 0) || (lo < 0))
+ hi = hex_to_bin(*src++);
+ if (unlikely(hi < 0))
+ return -EINVAL;
+ lo = hex_to_bin(*src++);
+ if (unlikely(lo < 0))
return -EINVAL;
*dst++ = (hi << 4) | lo;
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 20+ messages in thread