linux-kernel - Re: [PATCH] soundwire: sysfs_emit() expects a buffer of size PAGE

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aIICZmB5-7gj1M6W@vaman>
Date: Thu, 24 Jul 2025 15:22:38 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Takashi Iwai <tiwai@...e.de>
Cc: Sergio Perez Gonzalez <sperezglz@...il.com>,
	yung-chuan.liao@...ux.intel.com, pierre-louis.bossart@...ux.dev,
	linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soundwire: sysfs_emit() expects a buffer of size
 PAGE_SIZE

On 24-07-25, 09:19, Takashi Iwai wrote:
> On Thu, 24 Jul 2025 01:09:25 +0200,
> Sergio Perez Gonzalez wrote:
> > 
> > In read_buffer_show(), allocate sufficient memory to pass on to
> > sysfs_emit(), which expects a buffer of size PAGE_SIZE.
> > 
> > Link: https://scan7.scan.coverity.com/#/project-view/53936/11354?selectedIssue=1648019
> > Fixes: 35323d8ab811 ("soundwire: replace scnprintf() with sysfs_emit() for sysfs consistency")
> > Signed-off-by: Sergio Perez Gonzalez <sperezglz@...il.com>
> > ---
> >  drivers/soundwire/debugfs.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
> > index 825f7abbad32..08fb1e29026e 100644
> > --- a/drivers/soundwire/debugfs.c
> > +++ b/drivers/soundwire/debugfs.c
> > @@ -306,12 +306,12 @@ static int cmd_go(void *data, u64 value)
> >  DEFINE_DEBUGFS_ATTRIBUTE(cmd_go_fops, NULL,
> >  			 cmd_go, "%llu\n");
> >  
> > -#define MAX_LINE_LEN 128
> > -
> >  static int read_buffer_show(struct seq_file *s_file, void *data)
> >  {
> > -	char buf[MAX_LINE_LEN];
> >  	int i;
> > +	char *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
> 
> IMO, it's better to scratch the whole previous change.
> It makes little sense to allocate a large buffer here just for the
> temporary formatting.
> 
> Moreover, *_show() functions there take seq_file pointer, and you can
> just use seq_printf() directly.  The sysfs/kobject show callback is
> with the a fixed PAGE_SIZE buffer, hence sysfs_emit() is useful, but
> in this case, it's a completely different story.

Agree, dropped now!

-- 
~Vinod