netdev - Re: [PATCH] net: remove the unnecessary list

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: <20130225.031256.202114640375754800.davem@davemloft.net>
Date:	Mon, 25 Feb 2013 03:12:56 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	gaofeng@...fujitsu.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] net: remove the unnecessary list_del

From: Gao feng <gaofeng@...fujitsu.com>
Date: Mon, 25 Feb 2013 16:01:24 +0800

> I think it's better to make sure netdevice objects have valid pointers
> by calling list_del in unregister_netdevice_many.

Please, for the sake of understanding, read the commit below.  And
please make such necessary research in the future and reference any
such commits in your proposed patch, as well as explaining why those
commits in the past were wrong and why you're reverting of them is
correct.

Just saying "this isn't necessary" is a very disappointing explanation
for something as very non-trivial as this is.

Thank you.

commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
Author: Eric Dumazet <eric.dumazet@...il.com>
Date:   Thu Feb 17 22:59:19 2011 +0000

    net: deinit automatic LIST_HEAD
    
    commit 9b5e383c11b08784 (net: Introduce
    unregister_netdevice_many()) left an active LIST_HEAD() in
    rollback_registered(), with possible memory corruption.
    
    Even if device is freed without touching its unreg_list (and therefore
    touching the previous memory location holding LISTE_HEAD(single), better
    close the bug for good, since its really subtle.
    
    (Same fix for default_device_exit_batch() for completeness)
    
    Reported-by: Michal Hocko <mhocko@...e.cz>
    Tested-by: Michal Hocko <mhocko@...e.cz>
    Reported-by: Eric W. Biderman <ebiderman@...ssion.com>
    Tested-by: Eric W. Biderman <ebiderman@...ssion.com>
    Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
    Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
    CC: Ingo Molnar <mingo@...e.hu>
    CC: Octavian Purdila <opurdila@...acom.com>
    CC: stable <stable@...nel.org> [.33+]
    Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/net/core/dev.c b/net/core/dev.c
index a18c164..8ae6631 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5066,6 +5066,7 @@ static void rollback_registered(struct net_device *dev)
 
 	list_add(&dev->unreg_list, &single);
 	rollback_registered_many(&single);
+	list_del(&single);
 }
 
 unsigned long netdev_fix_features(unsigned long features, const char *name)
@@ -6219,6 +6220,7 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
 		}
 	}
 	unregister_netdevice_many(&dev_kill_list);
+	list_del(&dev_kill_list);
 	rtnl_unlock();
 }
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html