On Wed, Jun 9, 2021 at 8:19 PM Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: > > On Wed, Jun 09, 2021 at 10:10:53AM -0700, Lucas De Marchi 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. > > another thing... what is your git-sendemail setup? This is putting patch > 2 as a reply to patch 1 and that breaks b4. See: > https://lore.kernel.org/linux-modules/20210608062923.94017-1-ykaliuta@xxxxxxxxxx/T/#u Thanks, yes. I have thread = true, but send it after the cover letter with git send-email --to linux-modules@xxxxxxxxxxxxxxx --cc lucas.de.marchi@xxxxxxxxx --in-reply-to=20210608062859.93959-1-ykaliuta@xxxxxxxxxx 0001-libkmod-module-check-new_from_name-return-value-in-g.patch 0002-libkmod-builtin-consider-final-NIL-in-name-length-ch.patch So, the right way is to send all together, or disable 'thread'. > > Lucas De Marchi > > > > >thanks > >LUcas De Marchi > > > >> > >>Check the length against PATH_MAX - 1. > >> > >>Signed-off-by: Yauheni Kaliuta <ykaliuta@xxxxxxxxxx> > >>--- > >>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
