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: <gwdzoxk5gq5ve5jqklt4mkuoo25nhpjagjwbjuqvcrwhccauet@2frrkt22grg7>
Date: Wed, 15 Jan 2025 22:14:52 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Dennis Dalessandro <dennis.dalessandro@...nelisnetworks.com>
Cc: Vitaliy Shevtsov <v.shevtsov@...ima.ru>, lvc-project@...uxtesting.org, 
	Leon Romanovsky <leon@...nel.org>, linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jason Gunthorpe <jgg@...pe.ca>, Doug Ledford <dledford@...hat.com>
Subject: Re: [lvc-project] [PATCH] IB/hfi1: fix buffer underflow in fault
 injection code

On Wed, 15. Jan 12:55, Dennis Dalessandro wrote:
> On 12/27/24 6:09 PM, Vitaliy Shevtsov wrote:
> > [Why]
> > The fault injection code may have a buffer underflow, which may cause
> > memory corruption by writing a newline character before the base address of
> > the array. This can happen if the fault->opcodes bitmap is empty.
> > 
> > Since a file in debugfs is created with an empty bitmap, it is possible to
> > read the file before any set bits are written to it.
> > 
> > [How]
> > Fix this by checking that the size variable is greater than zero, otherwise
> > return zero as the number of bytes read.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with Svace.
> > 
> > Fixes: a74d5307caba ("IB/hfi1: Rework fault injection machinery")
> > Signed-off-by: Vitaliy Shevtsov <v.shevtsov@...ima.ru>
> > ---
> >  drivers/infiniband/hw/hfi1/fault.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/hw/hfi1/fault.c b/drivers/infiniband/hw/hfi1/fault.c
> > index ec9ee59fcf0c..2d87f9c8b89d 100644
> > --- a/drivers/infiniband/hw/hfi1/fault.c
> > +++ b/drivers/infiniband/hw/hfi1/fault.c
> > @@ -190,7 +190,8 @@ static ssize_t fault_opcodes_read(struct file *file, char __user *buf,
> >  		bit = find_next_bit(fault->opcodes, bitsize, zero);
> >  	}
> >  	debugfs_file_put(file->f_path.dentry);
> > -	data[size - 1] = '\n';
> > +	if (size)
> > +		data[size - 1] = '\n';
> >  	data[size] = '\0';
> >  	ret = simple_read_from_buffer(buf, len, pos, data, size);
> >  free_data:
> 
> I don't think size can ever be 0. No reason to change this I don't think.
> 
> -Denny
> 

Seems the patch description rather clearly shows the size can be zero in
case the corresponding opcodes bitmap is empty. Which is the case when
user reads the file before writing anything to it.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ