lkml.org 
[lkml]   [2019]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] net: check negative value for signed refcnt
From
Date
Hi, Alexandre,

On 31.01.2019 16:20, alexandre.besnard@softathome.com wrote:
> From: Alexandre Besnard <alexandre.besnard@softathome.com>
>
> Device remaining references counter is get as a signed integer.
>
> When unregistering network devices, the loop waiting for this counter
> to decrement tests the 0 strict equality. Thus if an error occurs and
> two references are given back by a protocol, we are stuck in the loop
> forever, with a -1 value.
>
> Robustness is added by checking a negative value: the device is then
> considered free of references, and a warning is issued (it should not
> happen, one should check that behavior)
>
> Signed-off-by: Alexandre Besnard <alexandre.besnard@softathome.com>
> ---
> net/core/dev.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ddc551f..e4190ae 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8687,6 +8687,11 @@ static void netdev_wait_allrefs(struct net_device *dev)
> refcnt = netdev_refcnt_read(dev);
>
> while (refcnt != 0) {
> + if (refcnt < 0) {
> + pr_warn("Device %s refcnt negative: device considered free, but it should not happen\n",
> + dev->name);
> + break;
> + }

1)I don't think this is a good approach. Negative value does not guarantee
there is just a double put of device reference. Negative value is an indicator
something goes wrong, and we definitely should not free device memory in
this case.

2)Not related to your patch -- it looks like we have problem in existing
code with this netdev_refcnt_read(). It does not imply a memory ordering
or some guarantees about reading percpu values. For example, in generic
code struct percpu_ref switches a counter into atomic mode before it checks
for the last reference. But there is nothing in netdev_refcnt_read().


\
 
 \ /
  Last update: 2019-01-31 14:51    [W:0.062 / U:1.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site