linux-kernel - Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next

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]

Date:	Thu, 24 Apr 2014 12:32:28 +0200
From:	Michal Malý <madcatxster@...oid-pointer.net>
To:	Oliver Neukum <oneukum@...e.de>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	dmitry.torokhov@...il.com, jkosina@...e.cz, elias.vds@...il.com,
	anssi.hannula@....fi, simon@...gewell.org
Subject: Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next

On Wednesday 23 of April 2014 15:41:03 Oliver Neukum wrote:
> On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
> >  static int drff_play(struct input_dev *dev, void *data,
> > 
> > -                                struct ff_effect *effect)
> > +                       const struct mlnx_effect_command *command)
> > 
> >  {
> >  
> >         struct hid_device *hid = input_get_drvdata(dev);
> >         struct drff_device *drff = data;
> > 
> > +       const struct mlnx_rumble_force *rumble_force =
> > &command->u.rumble_force;
> > 
> >         int strong, weak;
> > 
> > -       strong = effect->u.rumble.strong_magnitude;
> > -       weak = effect->u.rumble.weak_magnitude;
> > +       strong = rumble_force->strong;
> > +       weak = rumble_force->weak;
> > 
> >         dbg_hid("called with 0x%04x 0x%04x", strong, weak);
> > 
> > -       if (strong || weak) {
> > -               strong = strong * 0xff / 0xffff;
> > -               weak = weak * 0xff / 0xffff;
> > -
> > -               /* While reverse engineering this device, I found that
> > when
> > -                  this value is set, it causes the strong rumble to
> > function
> > -                  at a near maximum speed, so we'll bypass it. */
> > -               if (weak == 0x0a)
> > -                       weak = 0x0b;
> > -
> > -               drff->report->field[0]->value[0] = 0x51;
> > -               drff->report->field[0]->value[1] = 0x00;
> > -               drff->report->field[0]->value[2] = weak;
> > -               drff->report->field[0]->value[4] = strong;
> > -               hid_hw_request(hid, drff->report, HID_REQ_SET_REPORT);
> > -
> > -               drff->report->field[0]->value[0] = 0xfa;
> > -               drff->report->field[0]->value[1] = 0xfe;
> > -       } else {
> > +       switch (command->cmd) {
> > +       case MLNX_START_RUMBLE:
> > +               if (strong || weak) {
> > +                       strong = strong * 0xff / 0xffff;
> > +                       weak = weak * 0xff / 0xffff;
> > +
> > +                       /* While reverse engineering this device, I
> > found that when
> > +                       this value is set, it causes the strong rumble
> > to function
> > +                       at a near maximum speed, so we'll bypass it.
> > */
> > +                       if (weak == 0x0a)
> > +                               weak = 0x0b;
> > +
> > +                       drff->report->field[0]->value[0] = 0x51;
> > +                       drff->report->field[0]->value[1] = 0x00;
> > +                       drff->report->field[0]->value[2] = weak;
> > +                       drff->report->field[0]->value[4] = strong;
> 
> This looks like an endianness bug.

I don't have a big endian machine to check but why would this be an endianness 
issue? We're dealing with values all the time here, not addresses so I'd 
expect the 'weak' and 'strong' values to be truncated if they won't fit into 
byte. Division done beforehand makes sure that the values are within <0; 255> 
range. As far as I can see this is quite common in the HID and Input code. Am 
I missing something here?

Thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/