netdev - Re: [PATCH net] cxgb4: fix refcount init for TC-MQPRIO offload

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:   Wed, 18 Dec 2019 09:03:13 +0530
From:   Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, nirranjan@...lsio.com, vishal@...lsio.com,
        dt@...lsio.com
Subject: Re: [PATCH net] cxgb4: fix refcount init for TC-MQPRIO offload

On Tuesday, December 12/17/19, 2019 at 14:09:30 -0800, David Miller wrote:
> From: Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>
> Date: Mon, 16 Dec 2019 19:54:02 +0530
> 
> > @@ -205,7 +205,11 @@ static int cxgb4_mqprio_alloc_hw_resources(struct net_device *dev)
> >  			cxgb4_enable_rx(adap, &eorxq->rspq);
> >  	}
> >  
> > -	refcount_inc(&adap->tc_mqprio->refcnt);
> > +	if (!refcount_read(&adap->tc_mqprio->refcnt))
> > +		refcount_set(&adap->tc_mqprio->refcnt, 1);
> > +	else
> > +		refcount_inc(&adap->tc_mqprio->refcnt);
> > +
> 
> This is not correct.
> 
> You're bypassing the whole point of the refcount_t type which is to make sure
> that code that initializes a new object explicitly calls refcount_set() and
> that once the refcount goes to zero the object is freed or the refcount is
> initialized again using refcount_set() or similar.
> 
> I'm not applying this, it's just papering around the real problem.

Ok, I'll move refcount_set() closer to where the hardware queue
arrays, that it's tracking, have been allocated. Will send a v2.

Thanks,
Rahul