netdev - Re: [PATCH net-next v3] net: assign err to 0 at begin in do

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: <20171117.150825.1222925061943453941.davem@davemloft.net>
Date:   Fri, 17 Nov 2017 15:08:25 +0900 (KST)
From:   David Miller <davem@...emloft.net>
To:     cugyly@....com
Cc:     netdev@...r.kernel.org, Linyu.Yuan@...atel-sbell.com.cn
Subject: Re: [PATCH net-next v3] net: assign err to 0 at begin in
 do_setlink() function

From: yuan linyu <cugyly@....com>
Date: Thu, 16 Nov 2017 19:59:48 +0800

> From: yuan linyu <Linyu.Yuan@...atel-sbell.com.cn>
> 
> each netlink attribute have proper process when error happen,
> when exit one attribute process, it implies that no error,
> so err = 0; is useless.
> 
> assign err = 0; at beginning if all attributes not set.
> 
> v1 -> v2:
> 	fix review comment from David, clear err before
> 	nla_for_each_nested()
> 
> v2 -> v3:
> 	maybe wrong understanding of David comment,
> 	provide a new version
> 
> Signed-off-by: yuan linyu <Linyu.Yuan@...atel-sbell.com.cn>

I'm sorry I still find it hard to accept this change.

What about all of the assignments of 'err' which only branch to
'errout' if err is negative?  It is not easy to see that none of those
case ever result in 'err' holding a positive non-zero value.

The code as-is is the easiest to understand, audit and prove correct
in the error-free case.  And this because of the explicit clearing or
'err' to zero late in the function.

Thanks you.