All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frans Klaver <fransklaver@gmail.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Joe Perches <joe@perches.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kernel-janitors <kernel-janitors@vger.kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Yueyao Zhu <yueyao.zhu@gmail.com>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Guru Das Srinagesh <gurooodas@gmail.com>,
	Javier Martinez Canillas <javier@dowhile0.org>,
	devel@driverdev.osuosl.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: endian bitshift defects [ was: staging: fusb302: don't bitshift __le16 type ]
Date: Mon, 26 Jun 2017 10:06:14 +0200	[thread overview]
Message-ID: <CAH6sp9OCydowraafgmvp_BNe_Riqg3YA_fXm38m17nqFi49Tqw@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1706231936440.2989@hadrien>

On Sat, Jun 24, 2017 at 1:37 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 24 Jun 2017, Frans Klaver wrote:
>
>> Hm. For some reason the great mail filtering scheme decided to push
>> this past my inbox :-/
>>
>> On Sat, Jun 17, 2017 at 12:44 AM, Joe Perches <joe@perches.com> wrote:
>> > On Fri, 2017-06-16 at 19:45 +0200, Frans Klaver wrote:
>> >> The header field in struct pd_message is declared as an __le16 type. The
>> >> data in the message is supposed to be little endian. This means we don't
>> >> have to go and shift the individual bytes into position when we're
>> >> filling the buffer, we can just copy the contents right away. As an
>> >> added benefit we don't get fishy results on big endian systems anymore.
>> >
>> > Thanks for pointing this out.
>> >
>> > There are several instances of this class of error.
>>
>> There are other smells around __(le|be) types that show up in staging
>> that might be worth checking in the rest of the kernel as well. e.g.
>> converting to cpu and storing it back into itself (possibly with its
>> bytes reversed), direct assignments without conversion and what else
>> you might have. sparse obviously already flags anything fishy going on
>> with these types, but cannot distinguish between the classes of
>> errors. I'll need to acquaint myself with spatch a bit more to be able
>> to track that down.
>
> If you have concrete code examples, even fake ones, illustrating a class
> of problem, then that would be great.

I'll see if I can produce some somewhere this week.

Thanks,
Frans

WARNING: multiple messages have this Message-ID (diff)
From: Frans Klaver <fransklaver@gmail.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Joe Perches <joe@perches.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kernel-janitors <kernel-janitors@vger.kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Yueyao Zhu <yueyao.zhu@gmail.com>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Guru Das Srinagesh <gurooodas@gmail.com>,
	Javier Martinez Canillas <javier@dowhile0.org>,
	devel@driverdev.osuosl.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: endian bitshift defects [ was: staging: fusb302: don't bitshift __le16 type ]
Date: Mon, 26 Jun 2017 08:06:14 +0000	[thread overview]
Message-ID: <CAH6sp9OCydowraafgmvp_BNe_Riqg3YA_fXm38m17nqFi49Tqw@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1706231936440.2989@hadrien>

On Sat, Jun 24, 2017 at 1:37 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 24 Jun 2017, Frans Klaver wrote:
>
>> Hm. For some reason the great mail filtering scheme decided to push
>> this past my inbox :-/
>>
>> On Sat, Jun 17, 2017 at 12:44 AM, Joe Perches <joe@perches.com> wrote:
>> > On Fri, 2017-06-16 at 19:45 +0200, Frans Klaver wrote:
>> >> The header field in struct pd_message is declared as an __le16 type. The
>> >> data in the message is supposed to be little endian. This means we don't
>> >> have to go and shift the individual bytes into position when we're
>> >> filling the buffer, we can just copy the contents right away. As an
>> >> added benefit we don't get fishy results on big endian systems anymore.
>> >
>> > Thanks for pointing this out.
>> >
>> > There are several instances of this class of error.
>>
>> There are other smells around __(le|be) types that show up in staging
>> that might be worth checking in the rest of the kernel as well. e.g.
>> converting to cpu and storing it back into itself (possibly with its
>> bytes reversed), direct assignments without conversion and what else
>> you might have. sparse obviously already flags anything fishy going on
>> with these types, but cannot distinguish between the classes of
>> errors. I'll need to acquaint myself with spatch a bit more to be able
>> to track that down.
>
> If you have concrete code examples, even fake ones, illustrating a class
> of problem, then that would be great.

I'll see if I can produce some somewhere this week.

Thanks,
Frans

  reply	other threads:[~2017-06-26  8:06 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-16 17:45 [PATCH] staging: fusb302: don't bitshift __le16 type Frans Klaver
2017-06-16 18:11 ` Guenter Roeck
2017-06-16 22:44 ` endian bitshift defects [ was: staging: fusb302: don't bitshift __le16 type ] Joe Perches
2017-06-16 22:44   ` Joe Perches
2017-06-16 22:49   ` [Cocci] [Fwd: endian bitshift defects [ was: staging: fusb302: don't bitshift __le16 type ]] Joe Perches
2017-06-17  5:23   ` endian bitshift defects [ was: staging: fusb302: don't bitshift __le16 type ] Julia Lawall
2017-06-17  5:23     ` Julia Lawall
2017-06-17  5:50     ` Joe Perches
2017-06-17  5:50       ` Joe Perches
2017-06-17  6:00       ` Julia Lawall
2017-06-17  6:00         ` Julia Lawall
2017-06-17  6:23         ` Joe Perches
2017-06-17  6:23           ` Joe Perches
2017-06-17  6:26           ` Julia Lawall
2017-06-17  6:26             ` Julia Lawall
2017-06-23 22:29   ` Frans Klaver
2017-06-23 22:29     ` Frans Klaver
2017-06-23 23:37     ` Julia Lawall
2017-06-23 23:37       ` Julia Lawall
2017-06-26  8:06       ` Frans Klaver [this message]
2017-06-26  8:06         ` Frans Klaver
2017-06-26  9:39         ` Julia Lawall
2017-06-26  9:39           ` Julia Lawall
2017-06-26 20:57       ` Frans Klaver
2017-06-26 20:57         ` Frans Klaver
2017-06-26 21:03         ` Julia Lawall
2017-06-26 21:03           ` Julia Lawall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAH6sp9OCydowraafgmvp_BNe_Riqg3YA_fXm38m17nqFi49Tqw@mail.gmail.com \
    --to=fransklaver@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gurooodas@gmail.com \
    --cc=javier@dowhile0.org \
    --cc=joe@perches.com \
    --cc=julia.lawall@lip6.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=rmfrfs@gmail.com \
    --cc=yueyao.zhu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.