From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v2 next-next] net: hns: enet specifies a reference to dsaf References: <1452654880-28980-1-git-send-email-yankejian@huawei.com> <56970F48.9060504@huawei.com> <1452764635.2521.28.camel@linux.intel.com> From: "Yankejian (Hackim Yim)" Message-ID: <569F2DEC.9040209@huawei.com> Date: Wed, 20 Jan 2016 14:49:16 +0800 MIME-Version: 1.0 In-Reply-To: <1452764635.2521.28.camel@linux.intel.com> Content-Type: multipart/alternative; boundary="------------080403090005090006040408" To: Andy Shevchenko , Yisen Zhuang , davem@davemloft.net, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, catalin.marinas@arm.com, will.deacon@arm.com, huangdaode@hisilicon.com, liguozhu@huawei.com, arnd@arndb.de, fengguang.wu@intel.com, salil.mehta@huawei.com, lisheng011@huawei.com Cc: devicetree@vger.kernel.org, netdev@vger.kernel.org, linuxarm@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-ID: --------------080403090005090006040408 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit On 2016/1/14 17:43, Andy Shevchenko wrote: >> >>> > > --- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c >>> > > +++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c >>> > > @@ -847,6 +847,7 @@ static struct hnae_ae_ops hns_dsaf_ops = { >>> > > int hns_dsaf_ae_init(struct dsaf_device *dsaf_dev) >>> > > { >>> > > struct hnae_ae_dev *ae_dev = &dsaf_dev->ae_dev; >>> > > + static atomic_t id = ATOMIC_INIT(-1); >>> > > >>> > > switch (dsaf_dev->dsaf_ver) { >>> > > case AE_VERSION_1: >>> > > @@ -858,6 +859,9 @@ int hns_dsaf_ae_init(struct dsaf_device >>> > > *dsaf_dev) >>> > > default: >>> > > break; >>> > > } >>> > > + >>> > > + snprintf(ae_dev->name, AE_NAME_SIZE, "%s%d", >>> > > DSAF_DEVICE_NAME, >>> > > + (int)atomic_inc_return(&id)); > If you bind/unbind device enough times you may get an overflow and end > up with name of existing device (if you have 1+ of them in the system). > > To avoid such situation better to use IDA/IDR framework. > Hi, Andy Thanks again for your suggestion. It seems that using IDA/IDR framework is better, but all of the functions must be serialized by lock. And actually, we confirm that it won't be overflow in our analysis before we implemented it. Best Regards, Kejian Yan --------------080403090005090006040408 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

On 2016/1/14 17:43, Andy Shevchenko wrote:
 

> > --- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
> > +++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
> > @@ -847,6 +847,7 @@ static struct hnae_ae_ops hns_dsaf_ops = {
> >  int hns_dsaf_ae_init(struct dsaf_device *dsaf_dev)
> >  {
> >  	struct hnae_ae_dev *ae_dev = &dsaf_dev->ae_dev;
> > +	static atomic_t id = ATOMIC_INIT(-1);
> >  
> >  	switch (dsaf_dev->dsaf_ver) {
> >  	case AE_VERSION_1:
> > @@ -858,6 +859,9 @@ int hns_dsaf_ae_init(struct dsaf_device
> > *dsaf_dev)
> >  	default:
> >  		break;
> >  	}
> > +
> > +	snprintf(ae_dev->name, AE_NAME_SIZE, "%s%d",
> > DSAF_DEVICE_NAME,
> > +		 (int)atomic_inc_return(&id));

If you bind/unbind device enough times you may get an overflow and end
up with name of existing device (if you have 1+ of them in the system).

To avoid such situation better to use IDA/IDR framework.
Hi, Andy
Thanks again for your suggestion.
It seems that using IDA/IDR framework is better, but all of the functions must be serialized by lock.
And actually, we confirm that it won't be overflow in our analysis before we implemented it.

Best Regards,
Kejian Yan



--------------080403090005090006040408--