linux-kernel - Re: [PATCH 5.15] udf: Allocate name buffer in directory iterator on heap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241115012048.GK1458936@google.com>
Date: Fri, 15 Nov 2024 10:20:48 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Sasha Levin <sashal@...nel.org>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>, stable@...r.kernel.org,
	linux-kernel@...r.kernel.org, Jan Kara <jack@...e.cz>,
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH 5.15] udf: Allocate name buffer in directory iterator on
 heap

On (24/11/14 13:31), Sasha Levin wrote:
> On Wed, Nov 13, 2024 at 01:30:35PM +0900, Sergey Senozhatsky wrote:
> > From: Jan Kara <jack@...e.cz>
> > 
> > [ Upstream commit 0aba4860b0d0216a1a300484ff536171894d49d8 ]
> > 
> > Currently we allocate name buffer in directory iterators (struct
> > udf_fileident_iter) on stack. These structures are relatively large
> > (some 360 bytes on 64-bit architectures). For udf_rename() which needs
> > to keep three of these structures in parallel the stack usage becomes
> > rather heavy - 1536 bytes in total. Allocate the name buffer in the
> > iterator from heap to avoid excessive stack usage.
> > 
> > Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
> > Reported-by: kernel test robot <lkp@...el.com>
> > Signed-off-by: Jan Kara <jack@...e.cz>
> 
> Your S-O-B is missing, but also it doesn't build:

OK, didn't know that I need to add my SoB.

> fs/udf/directory.c: In function 'udf_fiiter_init':
> fs/udf/directory.c:251:25: error: implicit declaration of function 'kmalloc'; did you mean 'kvmalloc'? [-Werror=implicit-function-declaration]
>   251 |         iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
>       |                         ^~~~~~~
>       |                         kvmalloc
> fs/udf/directory.c:251:23: warning: assignment to 'uint8_t *' {aka 'unsigned char *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
>   251 |         iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
>       |                       ^
> fs/udf/directory.c: In function 'udf_fiiter_release':
> fs/udf/directory.c:315:9: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
>   315 |         kfree(iter->namebuf);
>       |         ^~~~~
>       |         kvfree

Hmm.  Upstream fs/udf/directory.c doesn't include slab.h and 5.15
with this patch applied "builds on my computer".  So I can amend
Jan's patch to include slab.h, I guess?  Is that okay?