netdev - Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq

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: <8cc2925b53845876475ccc4f098fbc88471d77bb.camel@perches.com>
Date:   Sun, 09 Aug 2020 20:54:24 -0700
From:   Joe Perches <joe@...ches.com>
To:     Shannon Nelson <snelson@...sando.io>, Xu Wang <vulab@...as.ac.cn>,
        drivers@...sando.io, davem@...emloft.net, kuba@...nel.org,
        netdev@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ionic_lif: Use devm_kcalloc() in ionic_qcq_alloc()

On Sun, 2020-08-09 at 20:50 -0700, Shannon Nelson wrote:
> On 8/9/20 8:20 PM, Joe Perches wrote:
> > On Mon, 2020-08-10 at 02:38 +0000, Xu Wang wrote:
> > > A multiplication for the size determination of a memory allocation
> > > indicated that an array data structure should be processed.
> > > Thus use the corresponding function "devm_kcalloc".
> > []
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > []
> > > @@ -412,7 +412,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> > >   
> > >   	new->flags = flags;
> > >   
> > > -	new->q.info = devm_kzalloc(dev, sizeof(*new->q.info) * num_descs,
> > > +	new->q.info = devm_kcalloc(dev, num_descs, sizeof(*new->q.info),
> > >   				   GFP_KERNEL);
> > >   	if (!new->q.info) {
> > >   		netdev_err(lif->netdev, "Cannot allocate queue info\n");
> > You could also remove these unnecessary allocation error messages.
> > There is an existing dump_stack() on allocation failure.
> > 
> Yes, the dump_stack() tells you which function had the allocation 
> failure, but since there are multiple allocation operations in this same 
> function, I find these helpful in knowing quickly which one of the 
> allocations failed, without having to track down the symbols and source 
> for whatever distro's kernel this might have happened in.

If you do chose the message, might as well add __GFP_NOWARN
to the allocation to avoid the dump_stack().

But honestly, if any allocation fails, you're OOM anyway.