linux-kernel - Re: [PATCH] staging: greybus: add logging statement when kfifo

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]

Date:   Thu, 11 Jul 2019 09:00:42 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Keyur Patel <iamkeyur96@...il.com>
Cc:     devel@...verdev.osuosl.org, Alex Elder <elder@...nel.org>,
        Johan Hovold <johan@...nel.org>, linux-kernel@...r.kernel.org,
        greybus-dev@...ts.linaro.org, David Lin <dtwlin@...il.com>
Subject: Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc
 fails

On Wed, Jul 10, 2019 at 10:24:06AM -0400, Keyur Patel wrote:
> On Wed, Jul 10, 2019 at 06:35:38PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote:
> > > Added missing logging statement when kfifo_alloc fails, to improve
> > > debugging.
> > > 
> > > Signed-off-by: Keyur Patel <iamkeyur96@...il.com>
> > > ---
> > >  drivers/staging/greybus/uart.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> > > index b3bffe91ae99..86a395ae177d 100644
> > > --- a/drivers/staging/greybus/uart.c
> > > +++ b/drivers/staging/greybus/uart.c
> > > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
> > >  
> > >  	retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE,
> > >  			     GFP_KERNEL);
> > > -	if (retval)
> > > +	if (retval) {
> > > +		pr_err("kfifo_alloc failed\n");
> > >  		goto exit_buf_free;
> > > +	}
> > 
> > You should have already gotten an error message from the log if this
> > fails, from the kmalloc_array() call failing, right?
> > 
> > So why is this needed?  We have been trying to remove these types of
> > messages and keep them in the "root" place where the failure happens.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Didn't notice that. I agree that this will result only into redundancy. 
> Quick look over files reveal that there are multiple places
> where people are using print statements after memory allocation fails. 
> Should I go ahead and send patches to remove those
> redundant print statements?

It all depends on where in the kernel those are.  Some subsystems want
minor cleanups like this, but most do not.  So unless the issue you find
is in the drivers/staging/ directory, I would not worry about it.

thanks,

greg k-h