All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Shevchenko <andy@kernel.org>,
	Mimi Zohar <zohar@linux.ibm.com>,
	device-mapper development <dm-devel@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mike Snitzer <msnitzer@redhat.com>,
	Milan Broz <gmazyland@gmail.com>
Subject: Re: [PATCH v2] hex2bin: fix access beyond string end
Date: Wed, 27 Apr 2022 16:49:59 +0200	[thread overview]
Message-ID: <CAHp75Vf=sEeoHwzb2RdVFsmZ_mWAyg2zdMKgtW9RQFM_bTMHPw@mail.gmail.com> (raw)
In-Reply-To: <alpine.LRH.2.02.2204271000020.1114@file01.intranet.prod.int.rdu2.redhat.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Andy Shevchenko <andy@kernel.org>,
	Mike Snitzer <msnitzer@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mimi Zohar <zohar@linux.ibm.com>,
	device-mapper development <dm-devel@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Milan Broz <gmazyland@gmail.com>
Subject: Re: [dm-devel] [PATCH v2] hex2bin: fix access beyond string end
Date: Wed, 27 Apr 2022 16:49:59 +0200	[thread overview]
Message-ID: <CAHp75Vf=sEeoHwzb2RdVFsmZ_mWAyg2zdMKgtW9RQFM_bTMHPw@mail.gmail.com> (raw)
In-Reply-To: <alpine.LRH.2.02.2204271000020.1114@file01.intranet.prod.int.rdu2.redhat.com>

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


  reply	other threads:[~2022-04-27 14:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24 20:48 [PATCH] hex2bin: fix access beyond string end Mikulas Patocka
2022-04-24 20:48 ` [dm-devel] " Mikulas Patocka
2022-04-26 10:37 ` Andy Shevchenko
2022-04-26 10:37   ` [dm-devel] " Andy Shevchenko
2022-04-26 12:07   ` [PATCH v2] " Mikulas Patocka
2022-04-26 12:07     ` [dm-devel] " Mikulas Patocka
2022-04-26 13:18     ` Andy Shevchenko
2022-04-26 13:18       ` [dm-devel] " Andy Shevchenko
2022-04-26 13:19     ` Andy Shevchenko
2022-04-26 13:19       ` [dm-devel] " Andy Shevchenko
2022-04-26 15:29       ` Mikulas Patocka
2022-04-26 15:29         ` [dm-devel] " Mikulas Patocka
2022-04-27 12:24         ` Andy Shevchenko
2022-04-27 12:24           ` [dm-devel] " Andy Shevchenko
2022-04-27 14:10           ` Mikulas Patocka
2022-04-27 14:10             ` [dm-devel] " Mikulas Patocka
2022-04-27 14:49             ` Andy Shevchenko [this message]
2022-04-27 14:49               ` Andy Shevchenko
2022-04-27 15:26               ` [PATCH v3] " Mikulas Patocka
2022-04-27 15:26                 ` [dm-devel] " Mikulas Patocka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHp75Vf=sEeoHwzb2RdVFsmZ_mWAyg2zdMKgtW9RQFM_bTMHPw@mail.gmail.com' \
    --to=andy.shevchenko@gmail.com \
    --cc=andy@kernel.org \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=msnitzer@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.