[PATCH] IRDA: Fix genlmsg_put() return value check.

[PATCH] IRDA: Fix genlmsg_put() return value check.

[Julius Volz]

Fix an incorrect return value check of genlmsg_put() in irda_nl_get_mode().
genlmsg_put() does not use ERR_PTR() to encode return values, it just
returns NULL on error.

Signed-off-by: Julius Volz <juliusv@google.com>
---
 net/irda/irnetlink.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/irda/irnetlink.c b/net/irda/irnetlink.c
index 9e1fb82..ea11cb4 100644
--- a/net/irda/irnetlink.c
+++ b/net/irda/irnetlink.c
@@ -101,8 +101,8 @@ static int irda_nl_get_mode(struct sk_buff *skb, struct genl_info *info)
 
 	hdr = genlmsg_put(msg, info->snd_pid, info->snd_seq,
 			  &irda_nl_family, 0,  IRDA_NL_CMD_GET_MODE);
-	if (IS_ERR(hdr)) {
-		ret = PTR_ERR(hdr);
+	if (hdr == NULL) {
+		ret = -ENOMEM;
 		goto err_out;
 	}
 
-- 
1.5.3.6

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[Patrick McHardy]

Julius Volz wrote:
> Fix an incorrect return value check of genlmsg_put() in irda_nl_get_mode().
> genlmsg_put() does not use ERR_PTR() to encode return values, it just
> returns NULL on error.
> 
> Signed-off-by: Julius Volz <juliusv@google.com>
> ---
>  net/irda/irnetlink.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/irda/irnetlink.c b/net/irda/irnetlink.c
> index 9e1fb82..ea11cb4 100644
> --- a/net/irda/irnetlink.c
> +++ b/net/irda/irnetlink.c
> @@ -101,8 +101,8 @@ static int irda_nl_get_mode(struct sk_buff *skb, struct genl_info *info)
>  
>  	hdr = genlmsg_put(msg, info->snd_pid, info->snd_seq,
>  			  &irda_nl_family, 0,  IRDA_NL_CMD_GET_MODE);
> -	if (IS_ERR(hdr)) {
> -		ret = PTR_ERR(hdr);
> +	if (hdr == NULL) {
> +		ret = -ENOMEM;
>  		goto err_out;
>  	}

Good catch, but the correct return value for insufficient
space in the skb is -ENOSPC.

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[samuel]

On Mon, 23 Jun 2008 21:43:31 +0200, Patrick McHardy <kaber@trash.net>

wrote:

> Julius Volz wrote:

>> Fix an incorrect return value check of genlmsg_put() in

> irda_nl_get_mode().

>> genlmsg_put() does not use ERR_PTR() to encode return values, it just

>> returns NULL on error.

>>

>> Signed-off-by: Julius Volz <juliusv@google.com>

>> ---

>>  net/irda/irnetlink.c |    4 ++--

>>  1 files changed, 2 insertions(+), 2 deletions(-)

>>

>> diff --git a/net/irda/irnetlink.c b/net/irda/irnetlink.c

>> index 9e1fb82..ea11cb4 100644

>> --- a/net/irda/irnetlink.c

>> +++ b/net/irda/irnetlink.c

>> @@ -101,8 +101,8 @@ static int irda_nl_get_mode(struct sk_buff *skb,

> struct genl_info *info)

>>

>>  	hdr = genlmsg_put(msg, info->snd_pid, info->snd_seq,

>>  			  &irda_nl_family, 0,  IRDA_NL_CMD_GET_MODE);

>> -	if (IS_ERR(hdr)) {

>> -		ret = PTR_ERR(hdr);

>> +	if (hdr == NULL) {

>> +		ret = -ENOMEM;

>>  		goto err_out;

>>  	}

> 

> Good catch, but the correct return value for insufficient

> space in the skb is -ENOSPC.

Thanks Julius, Patrick. I'll take this one and send it with

my next IrDA patch set.



Cheers,

Samuel.

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[Julius Volz]

On Mon, Jun 23, 2008, Patrick McHardy wrote:
> Good catch, but the correct return value for insufficient
> space in the skb is -ENOSPC.

Hm, seems we have many inconsistent values returned in exactly this
situation (in response to genlmsg_put), but none of them are -ENOSPC:

fs/dlm/netlink.c: -EINVAL
net/netlink/genetlink.c: -1
net/netlabel/netlabel_unlabeled.c: -ENOMEM
net/netlabel/netlabel_cipso_v4.c: -ENOMEM
net/netlabel/netlabel_mgmt.c: -ENOMEM
net/wireless/nl80211.c: -1
drivers/acpi/event.c: -ENOMEM
kernel/taskstats.c: -EINVAL

Seems like -ENOMEM is most common, don't know if that means it's
correct, though...

I was also using -ENOMEM in my Netlink code, so better convert all my
new uses to -ENOSPC?

Julius

-- 
Google Switzerland GmbH

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[Patrick McHardy]

Julius Volz wrote:
> On Mon, Jun 23, 2008, Patrick McHardy wrote:
>> Good catch, but the correct return value for insufficient
>> space in the skb is -ENOSPC.
> 
> Hm, seems we have many inconsistent values returned in exactly this
> situation (in response to genlmsg_put), but none of them are -ENOSPC:
> 
> fs/dlm/netlink.c: -EINVAL
> net/netlink/genetlink.c: -1
> net/netlabel/netlabel_unlabeled.c: -ENOMEM
> net/netlabel/netlabel_cipso_v4.c: -ENOMEM
> net/netlabel/netlabel_mgmt.c: -ENOMEM
> net/wireless/nl80211.c: -1
> drivers/acpi/event.c: -ENOMEM
> kernel/taskstats.c: -EINVAL
> 
> Seems like -ENOMEM is most common, don't know if that means it's
> correct, though...
> 
> I was also using -ENOMEM in my Netlink code, so better convert all my
> new uses to -ENOSPC?

Yes, all those should be using ENOSPC as well. nl80211 returns
inconsistent errors currently (-1/EMSGSIZE), but doesn't propagate
them back to userspace. That should probably be fixed as well.

Actually I was wrong, its EMSGSIZE, not ENOSPC that should be
returned, its more fitting and what other netlink users do as
well.

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[Julius Volz]

On Tue, Jun 24, 2008, Patrick McHardy wrote:
> Yes, all those should be using ENOSPC as well. nl80211 returns
> inconsistent errors currently (-1/EMSGSIZE), but doesn't propagate
> them back to userspace. That should probably be fixed as well.
>
> Actually I was wrong, its EMSGSIZE, not ENOSPC that should be
> returned, its more fitting and what other netlink users do as
> well.

Thanks, I will use that then!

Julius

-- 
Google Switzerland GmbH

[PATCH] IRDA: Fix genlmsg_put() return value check.

[Julius Volz]

Fix an incorrect return value check of genlmsg_put() in irda_nl_get_mode().
genlmsg_put() does not use ERR_PTR() to encode return values, it just
returns NULL on error.

Signed-off-by: Julius Volz <juliusv@google.com>
---
 net/irda/irnetlink.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/irda/irnetlink.c b/net/irda/irnetlink.c
index 9e1fb82..ea11cb4 100644
--- a/net/irda/irnetlink.c
+++ b/net/irda/irnetlink.c
@@ -101,8 +101,8 @@ static int irda_nl_get_mode(struct sk_buff *skb, struct genl_info *info)
 
 	hdr = genlmsg_put(msg, info->snd_pid, info->snd_seq,
 			  &irda_nl_family, 0,  IRDA_NL_CMD_GET_MODE);
-	if (IS_ERR(hdr)) {
-		ret = PTR_ERR(hdr);
+	if (hdr == NULL) {
+		ret = -EMSGSIZE;
 		goto err_out;
 	}
 
-- 
1.5.3.6

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[David Miller]

From: Julius Volz <juliusv@google.com>
Date: Tue, 24 Jun 2008 11:29:14 +0200

> Fix an incorrect return value check of genlmsg_put() in irda_nl_get_mode().
> genlmsg_put() does not use ERR_PTR() to encode return values, it just
> returns NULL on error.
> 
> Signed-off-by: Julius Volz <juliusv@google.com>

I'm assuming Samuel will grab this and send it to me in his
next IRDA merge to me.

Re: [PATCH] IRDA: Fix genlmsg_put() return value check.

[samuel]

On Fri, 27 Jun 2008 20:01:33 -0700 (PDT), David Miller

<davem@davemloft.net> wrote:

> From: Julius Volz <juliusv@google.com>

> Date: Tue, 24 Jun 2008 11:29:14 +0200

> 

>> Fix an incorrect return value check of genlmsg_put() in

> irda_nl_get_mode().

>> genlmsg_put() does not use ERR_PTR() to encode return values, it just

>> returns NULL on error.

>>

>> Signed-off-by: Julius Volz <juliusv@google.com>

> 

> I'm assuming Samuel will grab this and send it to me in his

> next IRDA merge to me.

That's right. This should happen this week.



Cheers,

Samuel.