linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jhih Ming Huang <fbihjmeric@gmail.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	fabioaiuto83@gmail.com, ross.schm.dev@gmail.com,
	maqianga@uniontech.com, marcocesati@gmail.com,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] rtw_security: fix cast to restricted __le32
Date: Mon, 14 Jun 2021 14:14:15 +0000	[thread overview]
Message-ID: <YMdkN9cft6KHcFn3@zeniv-ca.linux.org.uk> (raw)
In-Reply-To: <CAKgboZ8QUQpiinL0xCxUmcp6nEVU20jXkDXbrK_QisUMiLEo1A@mail.gmail.com>

On Mon, Jun 14, 2021 at 12:40:27AM +0800, Jhih Ming Huang wrote:
> On Sun, Jun 13, 2021 at 8:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jun 13, 2021 at 08:28:58PM +0800, Jhih-Ming Huang wrote:
> > > This patch fixes the sparse warning of fix cast to restricted __le32.
> > >
> > > Last month, there was a change for replacing private CRC-32 routines with
> > > in-kernel ones.
> > > In that patch, we replaced getcrc32 with crc32_le in calling le32_to_cpu.
> > > le32_to_cpu accepts __le32 type as arg, but crc32_le returns unsigned int.
> > > That how it introduced the sparse warning.
> >
> > As crc32_le returns a u32 which is in native-endian format, how can you
> > cast it to le32?  Why do you cast it to le32?  Isn't that going to be
> > incorrect for big endian systems?
> >
> > thanks,
> >
> > greg k-h
> 
> Thanks for the fast reply.
> Yes, you are right. I did not notice that le32_to_cpu already handles
> both of the cases.
> 
> So it seems the warning from sparse is false positives, am I right?

In a sense that on all architectures we would be ever likely to support
le32_to_cpu and cpu_to_le32 do the same bit-shuffling - yes.  In a sense
of having those used correctly it's not a false positive, though - it's
much easier to follow "this variable always hold native-endian, this -
little-endian" and watch for conversions done correctly than to count
the byteswaps and try to prove that it's either even for all execution
histories or odd for all execution histories.

IOW, there's a good reason for keeping separate cpu_to_le32 and le32_to_cpu
and not mixing them with each other - it's easier to prove correctness that
way *and* easier to look for endianness bugs.

  reply	other threads:[~2021-06-14 14:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-13 12:28 [PATCH v2] rtw_security: fix cast to restricted __le32 Jhih-Ming Huang
2021-06-13 12:34 ` Greg KH
2021-06-13 16:40   ` Jhih Ming Huang
2021-06-14 14:14     ` Al Viro [this message]
2021-06-14 15:27       ` Jhih Ming Huang
2021-06-14 17:03         ` Al Viro
2021-06-18 18:17           ` [PATCH v3] " Jhih-Ming Huang
2021-06-18 19:29             ` Al Viro
2021-06-19  7:52               ` [PATCH v4] " Jhih-Ming Huang
2021-06-21  8:19                 ` [PATCH v5] " Jhih-Ming Huang
2021-06-21 15:48                   ` [PATCH v6] " Jhih-Ming Huang
2021-06-21 15:51                   ` [PATCH v5] " Jhih-Ming Huang
2021-06-22  9:31                   ` David Laight
2021-07-04 10:31                     ` [PATCH v7] " Jhih-Ming Huang
2021-07-04 19:05                       ` Greg KH
2021-08-01 15:51                         ` Jhih-Ming Huang
2021-08-05 11:17                           ` Greg KH
2021-06-19  9:20               ` [PATCH v3] " Jhih-Ming Huang
2021-06-18 18:28           ` [PATCH v2] " Jhih Ming Huang

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=YMdkN9cft6KHcFn3@zeniv-ca.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=fabioaiuto83@gmail.com \
    --cc=fbihjmeric@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=maqianga@uniontech.com \
    --cc=marcocesati@gmail.com \
    --cc=ross.schm.dev@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).