Message-ID: <1477317406.8424.1.camel@crowfest.net>
Subject: Re: [PATCH 1/2] staging: vc04_services: Fix unportable cast in
 vchiq_copy_from_user
From: Michael Zoran <mzoran@crowfest.net>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: daniels@collabora.com, eric@anholt.net, noralf@tronnes.org,
        popcornmix@gmail.com, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org
Date: Mon, 24 Oct 2016 06:56:46 -0700
In-Reply-To: <20161024132341.GA20116@kroah.com>
References: <20161024052918.28205-1-mzoran@crowfest.net>
         <20161024132341.GA20116@kroah.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1893
Lines: 57

On Mon, 2016-10-24 at 15:23 +0200, Greg KH wrote:
> On Sun, Oct 23, 2016 at 10:29:18PM -0700, mzoran@crowfest.net wrote:
> > From: Michael Zoran <mzoran@crowfest.net>
> > 
> > Signed-off-by: Michael Zoran <mzoran@crowfest.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.