Re: [PATCH 2/2] libkmod-builtin: consider final NIL in name length check

[Lucas De Marchi <lucas.demarchi@intel.com>]

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@redhat.com/T/#u

Lucas De Marchi

>
>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
>>