All of lore.kernel.org
 help / color / mirror / Atom feed
From: arvind Yadav <arvind.yadav.cs@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>, linuxppc-dev@lists.ozlabs.org
Cc: "qiang.zhao@freescale.com" <qiang.zhao@freescale.com>,
	"viresh.kumar@linaro.org" <viresh.kumar@linaro.org>,
	"zajec5@gmail.com" <zajec5@gmail.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	Scott Wood <scott.wood@nxp.com>,
	"David.Laight@aculab.com" <David.Laight@aculab.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"scottwood@freescale.com" <scottwood@freescale.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linux@roeck-us.net" <linux@roeck-us.net>
Subject: Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
Date: Tue, 2 Aug 2016 21:04:38 +0530	[thread overview]
Message-ID: <57A0BD8E.9050305@gmail.com> (raw)
In-Reply-To: <2484583.95xFmO7xir@wuerfel>


[-- Attachment #1.1: Type: text/plain, Size: 1523 bytes --]



On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:
> On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:
>> On 08/01/2016 02:02 AM, Arnd Bergmann wrote:
>>>> diff --git a/include/linux/err.h b/include/linux/err.h
>>>> index 1e35588..c2a2789 100644
>>>> --- a/include/linux/err.h
>>>> +++ b/include/linux/err.h
>>>> @@ -18,7 +18,17 @@
>>>>   
>>>>   #ifndef __ASSEMBLY__
>>>>   
>>>> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
>>>> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
>>>> +
>>>> +static inline int is_error_check(unsigned long error)
>>> Please leave the existing macro alone. I think you were looking for
>>> something specific to the return code of qe_muram_alloc() function,
>>> so please add a helper in that subsystem if you need it, not in
>>> the generic header files.
>> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
>> problem is certain callers that store the return value in a u32.  Why
>> not just fix those callers to store it in unsigned long (at least until
>> error checking is done)?
>>
> Yes, that would also address another problem with code like
>
>           kfree((void *)ugeth->tx_bd_ring_offset[i]);
>
> which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
> that also holds the return value of qe_muram_alloc.
>
> 	Arnd
Yes, we will fix caller. Caller api is not safe on 64bit.
Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long.



[-- Attachment #1.2: Type: text/html, Size: 2373 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

WARNING: multiple messages have this Message-ID (diff)
From: arvind Yadav <arvind.yadav.cs@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>, linuxppc-dev@lists.ozlabs.org
Cc: Scott Wood <scott.wood@nxp.com>,
	"qiang.zhao@freescale.com" <qiang.zhao@freescale.com>,
	"viresh.kumar@linaro.org" <viresh.kumar@linaro.org>,
	"zajec5@gmail.com" <zajec5@gmail.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"David.Laight@aculab.com" <David.Laight@aculab.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"scottwood@freescale.com" <scottwood@freescale.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linux@roeck-us.net" <linux@roeck-us.net>
Subject: Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
Date: Tue, 2 Aug 2016 21:04:38 +0530	[thread overview]
Message-ID: <57A0BD8E.9050305@gmail.com> (raw)
In-Reply-To: <2484583.95xFmO7xir@wuerfel>

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]



On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:
> On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:
>> On 08/01/2016 02:02 AM, Arnd Bergmann wrote:
>>>> diff --git a/include/linux/err.h b/include/linux/err.h
>>>> index 1e35588..c2a2789 100644
>>>> --- a/include/linux/err.h
>>>> +++ b/include/linux/err.h
>>>> @@ -18,7 +18,17 @@
>>>>   
>>>>   #ifndef __ASSEMBLY__
>>>>   
>>>> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
>>>> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
>>>> +
>>>> +static inline int is_error_check(unsigned long error)
>>> Please leave the existing macro alone. I think you were looking for
>>> something specific to the return code of qe_muram_alloc() function,
>>> so please add a helper in that subsystem if you need it, not in
>>> the generic header files.
>> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
>> problem is certain callers that store the return value in a u32.  Why
>> not just fix those callers to store it in unsigned long (at least until
>> error checking is done)?
>>
> Yes, that would also address another problem with code like
>
>           kfree((void *)ugeth->tx_bd_ring_offset[i]);
>
> which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
> that also holds the return value of qe_muram_alloc.
>
> 	Arnd
Yes, we will fix caller. Caller api is not safe on 64bit.
Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long.



[-- Attachment #2: Type: text/html, Size: 2373 bytes --]

  reply	other threads:[~2016-08-02 15:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-31 11:18 [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems Arvind Yadav
2016-07-31 11:18 ` Arvind Yadav
2016-08-01  7:02 ` Arnd Bergmann
2016-08-01  7:02   ` Arnd Bergmann
2016-08-01 16:55   ` Scott Wood
2016-08-01 16:55     ` Scott Wood
2016-08-02  7:45     ` Arnd Bergmann
2016-08-02  7:45       ` Arnd Bergmann
2016-08-02 15:34       ` arvind Yadav [this message]
2016-08-02 15:34         ` arvind Yadav
2016-08-02 19:57         ` Scott Wood
2016-08-02 19:57           ` Scott Wood
2016-08-03 13:55           ` arvind Yadav
2016-08-02 15:48       ` arvind Yadav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57A0BD8E.9050305@gmail.com \
    --to=arvind.yadav.cs@gmail.com \
    --cc=David.Laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=qiang.zhao@freescale.com \
    --cc=scott.wood@nxp.com \
    --cc=scottwood@freescale.com \
    --cc=viresh.kumar@linaro.org \
    --cc=zajec5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.