netdev - Re: [rds-devel] [PATCH RFC RFC] rds: Use NETDEV_UNREGISTER in rds_tcp_dev_event() (then kill NETDEV_UNREGISTER

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:   Tue, 20 Mar 2018 09:29:27 -0400
From:   Sowmini Varadhan <sowmini.varadhan@...cle.com>
To:     H??kon Bugge <haakon.bugge@...cle.com>
Cc:     Kirill Tkhai <ktkhai@...tuozzo.com>, rds-devel@....oracle.com,
        OFED mailing list <linux-rdma@...r.kernel.org>,
        netdev@...r.kernel.org, edumazet@...gle.com, davem@...emloft.net
Subject: Re: [rds-devel] [PATCH RFC RFC] rds: Use NETDEV_UNREGISTER in
 rds_tcp_dev_event() (then kill NETDEV_UNREGISTER_FINAL)

On (03/20/18 12:37), H??kon Bugge wrote:
> 
> A little nit below. And some spelling issues in existing commentary
> you can consider fixing, since you reshuffle this file considerable.
> > +	if (net != &init_net && rtn->ctl_table)
> > +		kfree(rtn->ctl_table);
> 
> Well, this comes from the existing code, but as pointed out by Linus
> recently, kfree() handles NULL pointers. So, if rtn->ctl_table is
> NULL most likely, the code is OK _if_ you add an unlikely() around the
> if-clause. Otherwise, the ??? && rtn->ctl_table??? can simply be removed.

As you observe correctly, this comes from the existing code,
and is unrelated to the contents of the commit comment.

So if we feel passionately about htis, let us please fix this
separately, with its own ceremony.


> s/when/When/
> s/parameters,this/parameters, this/
> 
> Well, not part of your commit.

As above.
> 
> 
> >  * function  resets the RDS connections in that netns  so that we can
> 
> Two double spaces incidents above
> 
> Not part of your commit

As above.

Thanks much.
--Sowmini