Re: [PATCH] x86: Fixes warning: cast removes address space '__user' of expression in uaccess_64.h

[Dipendra Khadka]

I am sorry for the formatting of the text as it was due to changing to text
from html and also I wrote "why spare did not find the difference between
'long' and 'unsigned long' in this particular case." instead of "why Sparse
found the difference between 'long' and 'unsigned long' in this
particular case."

Thank you for your consideration.

On Sat, 18 Nov 2023 at 01:16, Dipendra Khadka <kdipendra88@xxxxxxxxx> wrote:
>
> Hi,
>
> I am not sure why spare did not find the difference between 'long' and
> 'unsigned long'
> in this particular case. I saw that in else case,there is use of
> unsigned long and sparse
> does not report a warning .Hence I thought casting to unsigned long
> will solve the problem.
> Also, there are not any other warnings thrown by spare in the uaccess_64.h file.
>
> I think casting x  to  'void __user *'before checking whether it is
> greater than or equal to zero
> in valid_user_address() will be more sensible and fix the warning either.
>
> Is this ok for you? Or have to cast to 'unsigned long' or other
> changes or no need to do anything?
>
>
>
> On Sat, 18 Nov 2023 at 00:00, Dipendra Khadka <kdipendra88@xxxxxxxxx> wrote:
> >
> > Hi,
> >
> > I am not sure why spare did not find the difference between 'long' and 'unsigned long'
> > in this particular case. I saw that in else case,there is use of unsigned long and sparse
> > does not report a warning .Hence I thought casting to unsigned long will solve the problem.
> > Also, there are not any other warnings thrown by spare in the uaccess_64.h file.
> >
> > I think casting x  to  'void __user *'before checking whether it is greater than or equal to zero
> > in valid_user_address() will be more sensible and fix the warning either.
> >
> > Is this ok for you? Or have to cast to 'unsigned long' or no need to do anything?
> >
> >
> > On Fri, 17 Nov 2023 at 21:04, Dave Hansen <dave.hansen@xxxxxxxxx> wrote:
> >>
> >> On 11/16/23 09:38, Dipendra Khadka wrote:
> >> > Sparse has identified a warning as follows:
> >> >
> >> > ./arch/x86/include/asm/uaccess_64.h:88:24: warning: cast removes address space '__user' of expression.
> >> >
> >> > Since the valid_user_address(x) macro implicitly casts the argument
> >> > to long and compares the converted value of x to zero, casting ptr
> >> > to unsigned long has no functional impact and does not trigger a
> >> > Sparse warning either.
> >>
> >> Why does sparse complain about a cast to 'long' but not 'unsigned long'?
> >>  Both remove the '__user' address space from the expression.  Were there
> >> just so many __user pointers being cast to 'unsigned long' that there's
> >> an exception in sparse for 'void __user *' => 'unsigned long'?
> >>
> >> Either way, if we're going to fix it it seems like it would be better to
> >> valid_user_address() actually handle, well, __user addresses rather than
> >> expecting callers to do it.