linux-kernel - Re: [PATCH] watchdog: add driver for StreamLabs USB watchdog device

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]

Date:	Wed, 16 Mar 2016 10:07:53 +0100
From:	Oliver Neukum <oneukum@...e.com>
To:	Alexey Klimov <klimov.linux@...il.com>
Cc:	Yury Norov <yury.norov@...il.com>, wim@...ana.be,
	Guenter Roeck <linux@...ck-us.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>,
	linux-watchdog@...r.kernel.org
Subject: Re: [PATCH] watchdog: add driver for StreamLabs USB watchdog device

On Wed, 2016-03-16 at 00:57 +0000, Alexey Klimov wrote:
> Hi Oliver,
> 
> On Thu, Mar 10, 2016 at 9:23 AM, Oliver Neukum <oneukum@...e.com> wrote:
> > On Thu, 2016-03-10 at 02:29 +0000, Alexey Klimov wrote:

> >> +     streamlabs_wdt->buffer[3] = 0x80;
> >> +
> >> +     streamlabs_wdt->buffer[6] = (timeout_msec & 0xff) << 8;
> >> +     streamlabs_wdt->buffer[7] = (timeout_msec & 0xff00) >> 8;
> >
> > We have macros for such conversions. Please use them.
> 
> I screwed here. It should be:
>     buffer[6] = timeout_msec & 0xff;
>     buffer[7] = (timeout_msec >> 8) & 0xff;
> 
> However, are you talking about using swab16() function? Or migrating
> to cpu_to_le() and friends functions?
> 
> If it's acceptable way to make buffer with u16 type?
> It slightly decreases lines of code and no conversion is needed here
> that way. I can do just without swapping bytes:
>  buffer[3] = timeout_msec;
> and that's it.

NO!

You cannot make assumptions about the byte order of the host. This goes
over the wire. It must be in a defined order. You need to use the
cpu_to_* family of macros which will do the right thing. You just
open coded them.

Sorry to be very blunt here, but we absolutely must not make assumptions
about which hosts USB drivers can run on.

	HTH
		Oliver