linux-kernel - Re: [PATCH v2 1/5] soc: qcom: smem: Use le32_to

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]

Message-ID: <20171005035902.GW1165@minitux>
Date:   Wed, 4 Oct 2017 20:59:02 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Chris Lew <clew@...eaurora.org>
Cc:     Stephen Boyd <sboyd@...eaurora.org>, andy.gross@...aro.org,
        david.brown@...aro.org, aneela@...eaurora.org,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/5] soc: qcom: smem: Use le32_to_cpu for partition
 size comparison

On Fri 15 Sep 13:53 PDT 2017, Chris Lew wrote:

> 
> 
> On 9/15/2017 11:39 AM, Stephen Boyd wrote:
> > On 09/14, Chris Lew wrote:
> > > Endianness can vary in the system, add le32_to_cpu when comparing
> > > size values from smem.
> > > 
> > > Signed-off-by: Chris Lew <clew@...eaurora.org>
> > > ---
> > > 
> > > Changes since v1:
> > > - New change
> > > 
> > >   drivers/soc/qcom/smem.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> > > index c28275be0038..db04c45d4132 100644
> > > --- a/drivers/soc/qcom/smem.c
> > > +++ b/drivers/soc/qcom/smem.c
> > > @@ -698,7 +698,7 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
> > >   			return -EINVAL;
> > >   		}
> > > -		if (header->size != entry->size) {
> > > +		if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
> > 
> > Also, it doesn't really matter. We're comparing two numbers with
> > the same endianness, so comparing them for equality before or
> > after swapping makes no difference. Sparse also (correctly)
> > doesn't complain here, because adding the conversion is not
> > necessary. Drop this patch?
> > 
> 
> Hey Bjorn, should we remove this patch? You had flagged this comparison in
> the first version of the global partition changes.
> 

As Stephen says; it's (technically) unnecessary.

But I think we should take this patch anyways, for the sake of saving
future readers of having to think about why there's one case in the
entire file where we don't do this translation.

Regards,
Bjorn