All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08  7:55 ` Wang Weidong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08  7:55 UTC (permalink / raw)
  To: David Miller, Vlad Yasevich, Neil Horman
  Cc: Daniel Borkmann, linux-sctp, netdev

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@huawei.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;
+	}
 	return 0;
 }
 
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08  7:55 ` Wang Weidong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08  7:55 UTC (permalink / raw)
  To: David Miller, Vlad Yasevich, Neil Horman
  Cc: Daniel Borkmann, linux-sctp, netdev

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@huawei.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;
+	}
 	return 0;
 }
 
-- 
1.7.12



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08  7:55 ` Wang Weidong
@ 2014-05-08  9:40   ` tingwei liu
  -1 siblings, 0 replies; 14+ messages in thread
From: tingwei liu @ 2014-05-08  9:40 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Neil Horman, Daniel Borkmann,
	linux-sctp, netdev, Tony

On Thu, May 8, 2014 at 3:55 PM, Wang Weidong <wangweidong1@huawei.com> 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@huawei.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;
> +       }
>         return 0;
>  }
>
Hope to resolve the problem of sctp_sysctl_net_unregister() in one patch!
kfree(table) should check net namesapce.
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index c82fdc1..844d2b0 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -459,7 +459,8 @@ void sctp_sysctl_net_unregister(struct net *net)

        table = net->sctp.sysctl_header->ctl_table_arg;
        unregister_net_sysctl_table(net->sctp.sysctl_header);
-       kfree(table);
+       if (!net_eq(net, &init_net))
+               kfree(table);
 }

 static struct ctl_table_header *sctp_sysctl_header;

Thanks
Tingwei Liu
> --
> 1.7.12
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08  9:40   ` tingwei liu
  0 siblings, 0 replies; 14+ messages in thread
From: tingwei liu @ 2014-05-08  9:40 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Neil Horman, Daniel Borkmann,
	linux-sctp, netdev, Tony

On Thu, May 8, 2014 at 3:55 PM, Wang Weidong <wangweidong1@huawei.com> 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@huawei.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;
> +       }
>         return 0;
>  }
>
Hope to resolve the problem of sctp_sysctl_net_unregister() in one patch!
kfree(table) should check net namesapce.
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index c82fdc1..844d2b0 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -459,7 +459,8 @@ void sctp_sysctl_net_unregister(struct net *net)

        table = net->sctp.sysctl_header->ctl_table_arg;
        unregister_net_sysctl_table(net->sctp.sysctl_header);
-       kfree(table);
+       if (!net_eq(net, &init_net))
+               kfree(table);
 }

 static struct ctl_table_header *sctp_sysctl_header;

Thanks
Tingwei Liu
> --
> 1.7.12
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08  7:55 ` Wang Weidong
@ 2014-05-08 11:11   ` Neil Horman
  -1 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2014-05-08 11:11 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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

>  	return 0;
>  }
>  
> -- 
> 1.7.12
> 
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08 11:11   ` Neil Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2014-05-08 11:11 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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

>  	return 0;
>  }
>  
> -- 
> 1.7.12
> 
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08 11:11   ` Neil Horman
@ 2014-05-08 11:26     ` Wang Weidong
  -1 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:26 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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 

>>  	return 0;
>>  }
>>  
>> -- 
>> 1.7.12
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08 11:26     ` Wang Weidong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:26 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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 

>>  	return 0;
>>  }
>>  
>> -- 
>> 1.7.12
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08 11:26     ` Wang Weidong
@ 2014-05-08 11:31       ` Wang Weidong
  -1 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:31 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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?

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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08 11:31       ` Wang Weidong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:31 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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?

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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08 11:31       ` Wang Weidong
@ 2014-05-08 11:41         ` Neil Horman
  -1 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2014-05-08 11:41 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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@vger.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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08 11:41         ` Neil Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2014-05-08 11:41 UTC (permalink / raw)
  To: Wang Weidong
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

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@huawei.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@vger.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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
  2014-05-08 11:41         ` Neil Horman
@ 2014-05-08 11:57           ` Wang Weidong
  -1 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:57 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

On 2014/5/8 19:41, Neil Horman wrote:
> 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@huawei.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
> 

Hm, Should I add this:

+	if (net->sctp.sysctl_header == NULL);
+		kfree(table);

to the revert patch or do a new patch?

Regards
Wang

>> 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@vger.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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register
@ 2014-05-08 11:57           ` Wang Weidong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Weidong @ 2014-05-08 11:57 UTC (permalink / raw)
  To: Neil Horman
  Cc: David Miller, Vlad Yasevich, Daniel Borkmann, linux-sctp, netdev

On 2014/5/8 19:41, Neil Horman wrote:
> 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@huawei.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
> 

Hm, Should I add this:

+	if (net->sctp.sysctl_header = NULL);
+		kfree(table);

to the revert patch or do a new patch?

Regards
Wang

>> 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@vger.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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> .
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-05-08 12:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-08  7:55 [PATCH net-next] sctp: add a checking for sctp_sysctl_net_register Wang Weidong
2014-05-08  7:55 ` Wang Weidong
2014-05-08  9:40 ` tingwei liu
2014-05-08  9:40   ` tingwei liu
2014-05-08 11:11 ` Neil Horman
2014-05-08 11:11   ` Neil Horman
2014-05-08 11:26   ` Wang Weidong
2014-05-08 11:26     ` Wang Weidong
2014-05-08 11:31     ` Wang Weidong
2014-05-08 11:31       ` Wang Weidong
2014-05-08 11:41       ` Neil Horman
2014-05-08 11:41         ` Neil Horman
2014-05-08 11:57         ` Wang Weidong
2014-05-08 11:57           ` Wang Weidong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.