Re:Re: [PATCH 3/3] dmaengine: xilinx: dpdma: Reserve space for a string terminator

[jiale yao]

At 2026-10-02 05:00:21, "Laurent Pinchart" <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>On Thu, Oct 01, 2026 at 04:50:23PM -0400, Frank Li wrote:
>> On Sat, Sep 26, 2026 at 05:34:15PM +0300, Laurent Pinchart wrote:
>> > On Sat, Sep 26, 2026 at 08:12:50PM +0800, Jiale Yao wrote:
>> > > xilinx_dpdma_debugfs_write() allocates a buffer of size bytes, and
>> > > strncpy_from_user() can fill it without a terminating NUL when the input
>> > > has no NUL in the copied range. strsep() and strcasecmp() then read
>> > > beyond the buffer.
>> >
>> > strncpy() has long been considered unsafe, and has finally been removed
>> > from the kernel in v7.2. A better fix would be to similarly replace
>> > strncpy_from_user() with a safe equivalent.
>> 
>> Do you means use strndup_user()?
>
>I was thinking about adding a strscpy_user(), but a dup version could
>possibly be interesting too if there are enough users that call
>k[zm]alloc + strncpy_from_user.

Thanks for all reviews, how about this one?
https://lore.kernel.org/all/20261003095922.575350-1-yaojiale02@xxxxxxx/
>
>> > > Allocate an extra byte and keep that byte zero-initialized, so the input
>> > > copied remains unchanged and the buffer is always terminated.
>> > >
>> > > Fixes: 1d220435cab3 ("dmaengine: xilinx: dpdma: Add debugfs support")
>> > > Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
>> > > ---
>> > >  drivers/dma/xilinx/xilinx_dpdma.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
>> > > index d9a3542c4531..b61ef3062d84 100644
>> > > --- a/drivers/dma/xilinx/xilinx_dpdma.c
>> > > +++ b/drivers/dma/xilinx/xilinx_dpdma.c
>> > > @@ -410,7 +410,7 @@ static ssize_t xilinx_dpdma_debugfs_write(struct file *f,
>> > >  	if (dpdma_debugfs.testcase != DPDMA_TC_NONE)
>> > >  		return -EBUSY;
>> > >
>> > > -	kern_buff = kzalloc(size, GFP_KERNEL);
>> > > +	kern_buff = kzalloc(size + 1, GFP_KERNEL);
>> > >  	if (!kern_buff)
>> > >  		return -ENOMEM;
>> > >  	kern_buff_start = kern_buff;
>
>-- 
>Regards,
>
>Laurent Pinchart