lib/hexdump.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
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
---
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;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.
...
> 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;
return hi;
> + lo = hex_to_bin(*src++);
> + if (lo < 0)
> return -EINVAL;
return lo;
> *dst++ = (hi << 4) | lo;
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.
--
With Best Regards,
Andy Shevchenko
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;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
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
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
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
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
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;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
© 2016 - 2026 Red Hat, Inc.