linux-kernel - Re: [PATCH] x86: Fixes warning: cast removes address space '__user' of expression in uaccess

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEKBCKMcdvgXrUnp-a+kKh1WsJ-1Weq3eaoH_ysS4-i9UJyrhw@mail.gmail.com>
Date: Sat, 23 Dec 2023 14:52:26 +0545
From: Dipendra Khadka <kdipendra88@...il.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, mjguzik@...il.com, 
	ira.weiny@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: Fixes warning: cast removes address space '__user'
 of expression in uaccess_64.h

Hi,

On Sat, 18 Nov 2023 at 01:32, Dipendra Khadka <kdipendra88@...il.com> wrote:
>
> 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@...il.com> 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?

I thought it would be a good time to ask again whether I need patching
or not for this warning?

Thanks

> >
> >
> >
> > On Sat, 18 Nov 2023 at 00:00, Dipendra Khadka <kdipendra88@...il.com> 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@...el.com> 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.
--
#Dipendra Khadka