Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Gerecke, Jason" <Jason.Gerecke@wacom.com>
To:     Pavel Machek <pavel@denx.de>
CC:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "Skomra, Aaron" <Aaron.Skomra@wacom.com>,
        Jiri Kosina <jikos@kernel.org>
Subject: Re: [PATCH 4.19 027/125] HID: wacom: generic: Treat serial number and
 related fields as unsigned
Thread-Topic: [PATCH 4.19 027/125] HID: wacom: generic: Treat serial number
 and related fields as unsigned
Thread-Index: AQHVmL+hfZWoSt7hxkmYkNPzu/4PaKeI7fcAgAAEIoCAAExzwQ==
Date:   Wed, 13 Nov 2019 15:43:07 +0000
Message-ID: <AM6PR07MB44548A58061624B037D7C60CED760@AM6PR07MB4454.eurprd07.prod.outlook.com>
References: <20191111181438.945353076@linuxfoundation.org>
 <20191111181444.186103315@linuxfoundation.org>
 <20191113104724.GD32553@amd>,<nycvar.YFH.7.76.1911131201010.1799@cbobk.fhfr.pm>
In-Reply-To: <nycvar.YFH.7.76.1911131201010.1799@cbobk.fhfr.pm>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: wacom.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6ec2b784-1aed-4943-1e9d-08d768502e0a
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Nov 2019 15:43:07.3131
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 9655a91b-107e-4537-834e-d15e84872626
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Zh9PTOcjHvOQN7NI87HJhtXuv9GLIPjkX7I/QcsqohHh9vMfhV6J5BVaS33tq0AKE00DNFBJtQPpCRHxog5BXw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR07MB4037
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Jiri Kosina <jikos@kernel.org>=0A=
> Sent: Wednesday, November 13, 2019 3:02 AM=0A=
> To: Pavel Machek <pavel@denx.de>=0A=
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.ke=
rnel.org <linux-kernel@vger.kernel.org>; stable@vger.kernel.org <stable@vge=
r.kernel.org>; Gerecke, Jason <Jason.Gerecke@wacom.com>; Skomra, Aaron <Aar=
on.Skomra@wacom.com>=0A=
> Subject: Re: [PATCH 4.19 027/125] HID: wacom: generic: Treat serial numbe=
r and related fields as unsigned=0A=
>  =0A=
> =0A=
> [EXTERNAL]=0A=
> =0A=
> On Wed, 13 Nov 2019, Pavel Machek wrote:=0A=
> =0A=
> > > From: Jason Gerecke <killertofu@gmail.com>=0A=
> > >=0A=
> > > commit ff479731c3859609530416a18ddb3db5db019b66 upstream.=0A=
> > >=0A=
> > > The HID descriptors for most Wacom devices oddly declare the serial=
=0A=
> > > number and other related fields as signed integers. When these number=
s=0A=
> > > are ingested by the HID subsystem, they are automatically sign-extend=
ed=0A=
> > > into 32-bit integers. We treat the fields as unsigned elsewhere in th=
e=0A=
> > > kernel and userspace, however, so this sign-extension causes problems=
.=0A=
> > > In particular, the sign-extended tool ID sent to userspace as ABS_MIS=
C=0A=
> > > does not properly match unsigned IDs used by xf86-input-wacom and lib=
wacom.=0A=
> > >=0A=
> > > We introduce a function 'wacom_s32tou' that can undo the automatic si=
gn=0A=
> > > extension performed by 'hid_snto32'. We call this function when proce=
ssing=0A=
> > > the serial number and related fields to ensure that we are dealing wi=
th=0A=
> > > and reporting the unsigned form. We opt to use this method rather tha=
n=0A=
> > > adding a descriptor fixup in 'wacom_hid_usage_quirk' since it should =
be=0A=
> > > more robust in the face of future devices.=0A=
> >=0A=
> > > +++ b/drivers/hid/wacom.h=0A=
> > > @@ -205,6 +205,21 @@ static inline void wacom_schedule_work(s=0A=
> > >     }=0A=
> > >  }=0A=
> > >=0A=
> > > +/*=0A=
> > > + * Convert a signed 32-bit integer to an unsigned n-bit integer. Und=
oes=0A=
> > > + * the normally-helpful work of 'hid_snto32' for fields that use sig=
ned=0A=
> > > + * ranges for questionable reasons.=0A=
> > > + */=0A=
> > > +static inline __u32 wacom_s32tou(s32 value, __u8 n)=0A=
> > > +{=0A=
> > > +   switch (n) {=0A=
> > > +   case 8:  return ((__u8)value);=0A=
> > > +   case 16: return ((__u16)value);=0A=
> > > +   case 32: return ((__u32)value);=0A=
> > > +   }=0A=
> > > +   return value & (1 << (n - 1)) ? value & (~(~0U << n)) : value;=0A=
> > > +}=0A=
> >=0A=
> > Can we do something like:=0A=
> >=0A=
=0A=
The implementation here was copied from 'hid_snto32' (except, of course,=0A=
to make it convert signed to unsigned). That function provides=0A=
justification for how it is written, but if changes make sense then=0A=
we should probably do them to both this and 'hid_snto32'.=0A=
=0A=
Jason=0A=
=0A=
> >     BUG_ON(n>32);=0A=
> =0A=
> Please no BUG_ON()s in bitop helpers.=0A=
> =0A=
> Thanks,=0A=
> =0A=
> --=0A=
> Jiri Kosina=0A=
> SUSE Labs=0A=
> =