From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-x244.google.com (mail-pa0-x244.google.com [IPv6:2607:f8b0:400e:c03::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3s3gbp1DhPzDqQ3 for ; Wed, 3 Aug 2016 01:48:54 +1000 (AEST) Received: by mail-pa0-x244.google.com with SMTP id hh10so12242305pac.1 for ; Tue, 02 Aug 2016 08:48:54 -0700 (PDT) Subject: Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems. To: Arnd Bergmann , linuxppc-dev@lists.ozlabs.org References: <1469963924-8800-1-git-send-email-arvind.yadav.cs@gmail.com> <1956647.cOmaJREgOE@wuerfel> <2484583.95xFmO7xir@wuerfel> Cc: Scott Wood , "qiang.zhao@freescale.com" , "viresh.kumar@linaro.org" , "zajec5@gmail.com" , "linux-wireless@vger.kernel.org" , "David.Laight@aculab.com" , "netdev@vger.kernel.org" , "scottwood@freescale.com" , "akpm@linux-foundation.org" , "davem@davemloft.net" , "linux@roeck-us.net" From: arvind Yadav Message-ID: <57A0C0DE.3090706@gmail.com> Date: Tue, 2 Aug 2016 21:18:46 +0530 MIME-Version: 1.0 In-Reply-To: <2484583.95xFmO7xir@wuerfel> Content-Type: text/plain; charset=windows-1252; format=flowed List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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. Need to work on it. Arvind