linux-kernel - Re: [PATCH 1/2] staging: vc04_services: Fix unportable cast in vchiq_copy_from

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1477317406.8424.1.camel@crowfest.net>
Date:   Mon, 24 Oct 2016 06:56:46 -0700
From:   Michael Zoran <mzoran@...wfest.net>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     daniels@...labora.com, eric@...olt.net, noralf@...nnes.org,
        popcornmix@...il.com, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] staging: vc04_services: Fix unportable cast in
 vchiq_copy_from_user

On Mon, 2016-10-24 at 15:23 +0200, Greg KH wrote:
> On Sun, Oct 23, 2016 at 10:29:18PM -0700, mzoran@...wfest.net wrote:
> > From: Michael Zoran <mzoran@...wfest.net>
> > 
> > Signed-off-by: Michael Zoran <mzoran@...wfest.net>
> > ---
> >  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git
> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > index 32d12e6..98c6819 100644
> > ---
> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > +++
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > @@ -219,7 +219,7 @@ remote_event_signal(REMOTE_EVENT_T *event)
> >  int
> >  vchiq_copy_from_user(void *dst, const void *src, int size)
> >  {
> > -	if ((uint32_t)src < TASK_SIZE) {
> > +	if ((unsigned long)src < TASK_SIZE) {
> >  		return copy_from_user(dst, src, size);
> >  	} else {
> >  		memcpy(dst, src, size);
> 
> Ick, that's horrid.
> 
> And I can't take patches without a changelog text.
> 
> Please fix up the callers to do the right thing, this shouldn't be a
> wrapper function at all, especially given the mess of a cast as Dan
> points out.  Just call the correct copy_from_user() call and handle
> the
> correct error return value.
> 
> thanks,
> 
> greg k-h

OK, this is getting a wee bit outside the scope of checking in arm64
fixes. I was simply invited to submit my arm64 specific changes to the
stagging tree.    

I'm starting to wonder if perhaps the "proper" maintainers should take
over this issue at this point.  Perhaps some of the people that were
involved in checking this into the upstream tree to begin with.

I'm new to the whole kernel source code and we are getting into
architecture issues.