[patch] netfilter: nfnetlink: guard against undefined entries

[patch] netfilter: nfnetlink: guard against undefined entries

[Jan Engelhardt]

parent a391d495f6082f6348b9074214e81c7e2dc1151c (v2.6.37-rc1-184-ga391d49)
commit 6e97eb121bc8e39c651645c3d937f468d2b3e7fe
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Sat Nov 6 21:19:16 2010 +0100

netfilter: nfnetlink: guard against undefined entries

Check for empty entries in struct nfnl_callback[] to avoid potential
NULL deference. (Because I have run into one during development.)

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/nfnetlink.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index b4a4532..7012882 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -152,7 +152,7 @@ replay:
 	}
 
 	nc = nfnetlink_find_client(type, ss);
-	if (!nc)
+	if (nc == NULL || nc->call == NULL)
 		return -EINVAL;
 
 	{
-- 
# Created with git-export-patch

Re: [patch] netfilter: nfnetlink: guard against undefined entries

[Patrick McHardy]

On 10.11.2010 02:50, Jan Engelhardt wrote:
> parent a391d495f6082f6348b9074214e81c7e2dc1151c (v2.6.37-rc1-184-ga391d49)
> commit 6e97eb121bc8e39c651645c3d937f468d2b3e7fe
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date:   Sat Nov 6 21:19:16 2010 +0100
> 
> netfilter: nfnetlink: guard against undefined entries
> 
> Check for empty entries in struct nfnl_callback[] to avoid potential
> NULL deference. (Because I have run into one during development.)

It seems silly to add checks for easy to find bugs in users of
this code. Finding the cause of -EINVAL is much harder than
getting a nice oops.

> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  net/netfilter/nfnetlink.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
> index b4a4532..7012882 100644
> --- a/net/netfilter/nfnetlink.c
> +++ b/net/netfilter/nfnetlink.c
> @@ -152,7 +152,7 @@ replay:
>  	}
>  
>  	nc = nfnetlink_find_client(type, ss);
> -	if (!nc)
> +	if (nc == NULL || nc->call == NULL)
>  		return -EINVAL;
>  
>  	{

Re: [patch] netfilter: nfnetlink: guard against undefined entries

[Jan Engelhardt]

On Thursday 2010-11-11 11:34, Patrick McHardy wrote:

>On 10.11.2010 02:50, Jan Engelhardt wrote:
>> parent a391d495f6082f6348b9074214e81c7e2dc1151c (v2.6.37-rc1-184-ga391d49)
>> commit 6e97eb121bc8e39c651645c3d937f468d2b3e7fe
>> Author: Jan Engelhardt <jengelh@medozas.de>
>> Date:   Sat Nov 6 21:19:16 2010 +0100
>> 
>> netfilter: nfnetlink: guard against undefined entries
>> 
>> Check for empty entries in struct nfnl_callback[] to avoid potential
>> NULL deference. (Because I have run into one during development.)
>
>It seems silly to add checks for easy to find bugs in users of
>this code. Finding the cause of -EINVAL is much harder than
>getting a nice oops.

But costs a reboot.
How about a printk?

>> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
>> index b4a4532..7012882 100644
>> --- a/net/netfilter/nfnetlink.c
>> +++ b/net/netfilter/nfnetlink.c
>> @@ -152,7 +152,7 @@ replay:
>>  	}
>>  
>>  	nc = nfnetlink_find_client(type, ss);
>> -	if (!nc)
>> +	if (nc == NULL || nc->call == NULL)
>>  		return -EINVAL;
>>  
>>  	{

Re: [patch] netfilter: nfnetlink: guard against undefined entries

[Patrick McHardy]

On 11.11.2010 21:56, Jan Engelhardt wrote:
> On Thursday 2010-11-11 11:34, Patrick McHardy wrote:
> 
>> On 10.11.2010 02:50, Jan Engelhardt wrote:
>>> parent a391d495f6082f6348b9074214e81c7e2dc1151c (v2.6.37-rc1-184-ga391d49)
>>> commit 6e97eb121bc8e39c651645c3d937f468d2b3e7fe
>>> Author: Jan Engelhardt <jengelh@medozas.de>
>>> Date:   Sat Nov 6 21:19:16 2010 +0100
>>>
>>> netfilter: nfnetlink: guard against undefined entries
>>>
>>> Check for empty entries in struct nfnl_callback[] to avoid potential
>>> NULL deference. (Because I have run into one during development.)
>>
>> It seems silly to add checks for easy to find bugs in users of
>> this code. Finding the cause of -EINVAL is much harder than
>> getting a nice oops.
> 
> But costs a reboot.

A forced unload of the nfnetlink module will release the mutex.

> How about a printk?

We usually don't protect against API misuse. There is an endless
amount of mistakes you can make that will cause oopses.