netdev - Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net

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]

Date:	Thu, 8 May 2014 07:41:24 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Wang Weidong <wangweidong1@...wei.com>
Cc:	David Miller <davem@...emloft.net>,
	Vlad Yasevich <vyasevich@...il.com>,
	Daniel Borkmann <dborkman@...hat.com>,
	linux-sctp@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] sctp: add a checking for
 sctp_sysctl_net_register

On Thu, May 08, 2014 at 07:31:09PM +0800, Wang Weidong wrote:
> On 2014/5/8 19:26, Wang Weidong wrote:
> > On 2014/5/8 19:11, Neil Horman wrote:
> >> On Thu, May 08, 2014 at 03:55:05PM +0800, Wang Weidong wrote:
> >>> When register_net_sysctl failed, we should free the
> >>> sysctl_table while the net_namespace is not init_net.
> >>>
> >>> Signed-off-by: Wang Weidong <wangweidong1@...wei.com>
> >>> ---
> >>>  net/sctp/sysctl.c | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> >>> index 844d2b0..4c17694 100644
> >>> --- a/net/sctp/sysctl.c
> >>> +++ b/net/sctp/sysctl.c
> >>> @@ -450,6 +450,11 @@ int sctp_sysctl_net_register(struct net *net)
> >>>  	}
> >>>  
> >>>  	net->sctp.sysctl_header = register_net_sysctl(net, "net/sctp", table);
> >>> +	if (net->sctp.sysctl_header == NULL) {
> >>> +		if (!net_eq(net, &init_net))
> >>> +			kfree(table);
> >>> +		return -ENOMEM;
> >>> +	}
> >> Sorry, we're not special casing this in 3 places.  Why not just revert the
> >> origonal commit so that all the sysctl tables can be handled the same way
> >> Neil
> >>
> > Hi Neil,
> > 
> > The original commit avoids to kmemdup 
> > 
> Hi Neil,
> 
> Sorry for the accomplished mail.
> Although, revert the original commit, the checking for net->sctp.sysctl_header
> is need from coding view.
> 
> What do you think?
> 
That was my review.  I get that the origional commit avoids kmemdup for the
init_net case.  My point was that, in so doing you created the need to special
case the handling of the sysctl table for init_net in 3 places now.  It seems to
me that, while its always nice to save space, it would be cleaner programming to
just accept the need for the extra 1.6Kb of space and not have to worry about
the special casing.

Neil

> Regards
> Wang
> 
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -- 
> >>> 1.7.12
> >>>
> >>>
> >>>
> >> --
> >> 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
> >>
> >>
> > 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
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