linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yauheni Kaliuta <ykaliuta@redhat.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: linux-modules@vger.kernel.org, lucas.de.marchi@gmail.com
Subject: Re: [PATCH 2/2] libkmod-builtin: consider final NIL in name length check
Date: Tue, 22 Jun 2021 09:22:40 +0300	[thread overview]
Message-ID: <CANoWswki6jcArKf4Q54_acTbR7k9iVHYW8Kv+-ZoRMoZLRaOsA@mail.gmail.com> (raw)
In-Reply-To: <20210609171053.c72kzwcdkdrgcwpv@ldmartin-desk2>

Hi, Lucas!

My filters missed your replies, sorry :(


On Wed, Jun 9, 2021 at 8:11 PM Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> On Tue, Jun 08, 2021 at 09:29:23AM +0300, Yauheni Kaliuta wrote:
> >There is potential buffer overrun in kmod_builtin_iter_get_modname()
> >for the name of length PATH_MAX. The required buffer size is
> >PATH_MAX, so `modname[len] = '\0'` with len == PATH_MAX will write
> >right beyond the buffer.
>
> this doesn't look correct. "with len == PATH_MAX" we will actually
> return an error.
>
> What indeed is happening is truncation: since we are not reserving 1
> char for NUL termination, we will truncate the name. If we update the
> commit message to state the right reasoning, then we can land this patch.
>
> I don't see any buffer overflow here, but I may be missing something.

Yes, you are right. I see the actual overflow is fixed by
1cab02ecf6ee ("libkmod: fix an overflow with wrong modules.builtin.modinfo")

I should have checked some reports more carefully. Please, ignore the patch.

> thanks
> LUcas De Marchi
>
> >
> >Check the length against PATH_MAX - 1.
> >
> >Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
> >---
> > libkmod/libkmod-builtin.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/libkmod/libkmod-builtin.c b/libkmod/libkmod-builtin.c
> >index a002cb5ee2c6..3d4d77ab29b3 100644
> >--- a/libkmod/libkmod-builtin.c
> >+++ b/libkmod/libkmod-builtin.c
> >@@ -246,7 +246,7 @@ bool kmod_builtin_iter_get_modname(struct kmod_builtin_iter *iter,
> >
> >       len = dot - line;
> >
> >-      if (len >= PATH_MAX) {
> >+      if (len >= PATH_MAX - 1) {
> >               sv_errno = ENAMETOOLONG;
> >               goto fail;
> >       }
> >--
> >2.31.1
> >
>


--
WBR, Yauheni


  parent reply	other threads:[~2021-06-22  6:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08  6:28 [PATCH 0/2] Fix a couple of potential problem (static analysis) Yauheni Kaliuta
2021-06-08  6:29 ` [PATCH 1/2] libkmod-module: check "new_from_name" return value in get_builtin Yauheni Kaliuta
2021-06-08  6:29   ` [PATCH 2/2] libkmod-builtin: consider final NIL in name length check Yauheni Kaliuta
2021-06-09 17:10     ` Lucas De Marchi
2021-06-09 17:18       ` Lucas De Marchi
2021-06-22  6:31         ` Yauheni Kaliuta
2021-06-22  6:22       ` Yauheni Kaliuta [this message]
2021-06-09 17:22   ` [PATCH 1/2] libkmod-module: check "new_from_name" return value in get_builtin Lucas De Marchi

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=CANoWswki6jcArKf4Q54_acTbR7k9iVHYW8Kv+-ZoRMoZLRaOsA@mail.gmail.com \
    --to=ykaliuta@redhat.com \
    --cc=linux-modules@vger.kernel.org \
    --cc=lucas.de.marchi@gmail.com \
    --cc=lucas.demarchi@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).