2019-06-27 03:27:23

by Jiunn Chang

[permalink] [raw]
Subject: [Linux-kernel-mentees][PATCH v2] nl80211: Fix undefined behavior in bit shift

Shifting signed 32-bit value by 31 bits is undefined. Changing most
significant bit to unsigned.

Changes included in v2:
- use subsystem specific subject lines
- CC required mailing lists

Signed-off-by: Jiunn Chang <[email protected]>
---
include/uapi/linux/nl80211.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6f09d1500960..fa7ebbc6ff27 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5314,7 +5314,7 @@ enum nl80211_feature_flags {
NL80211_FEATURE_TDLS_CHANNEL_SWITCH = 1 << 28,
NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR = 1 << 29,
NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR = 1 << 30,
- NL80211_FEATURE_ND_RANDOM_MAC_ADDR = 1 << 31,
+ NL80211_FEATURE_ND_RANDOM_MAC_ADDR = 1U << 31,
};

/**
--
2.22.0


2019-06-27 05:06:17

by Jiunn Chang

[permalink] [raw]
Subject: [Linux-kernel-mentees][PATCH v3] nl80211: Fix undefined behavior in bit shift

Shifting signed 32-bit value by 31 bits is undefined. Changing most
significant bit to unsigned.

Signed-off-by: Jiunn Chang <[email protected]>
---
Changes included in v3:
- remove change log from patch description

Changes included in v2:
- use subsystem specific subject lines
- CC required mailing lists

include/uapi/linux/nl80211.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6f09d1500960..fa7ebbc6ff27 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5314,7 +5314,7 @@ enum nl80211_feature_flags {
NL80211_FEATURE_TDLS_CHANNEL_SWITCH = 1 << 28,
NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR = 1 << 29,
NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR = 1 << 30,
- NL80211_FEATURE_ND_RANDOM_MAC_ADDR = 1 << 31,
+ NL80211_FEATURE_ND_RANDOM_MAC_ADDR = 1U << 31,
};

/**
--
2.22.0

2019-06-28 13:57:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [Linux-kernel-mentees][PATCH v2] nl80211: Fix undefined behavior in bit shift

On Wed, 2019-06-26 at 21:34 -0600, Shuah Khan wrote:
> On 6/26/19 9:25 PM, Jiunn Chang wrote:
> > Shifting signed 32-bit value by 31 bits is undefined. Changing most
> > significant bit to unsigned.
> >
> > Changes included in v2:
> > - use subsystem specific subject lines
> > - CC required mailing lists
> >
> > Signed-off-by: Jiunn Chang <[email protected]>
> > ---
>
> Move version change lines here. They don't belong in the commit log.

FWIW, in many cases people (maintainers) now *do* want them in the
commit log. Here, they're just editorial, so agree, but if real
technical changes were made due to reviews, they can indeed be useful.

johannes

2019-06-28 15:05:59

by Shuah Khan

[permalink] [raw]
Subject: Re: [Linux-kernel-mentees][PATCH v2] nl80211: Fix undefined behavior in bit shift

On 6/28/19 7:57 AM, Johannes Berg wrote:
> On Wed, 2019-06-26 at 21:34 -0600, Shuah Khan wrote:
>> On 6/26/19 9:25 PM, Jiunn Chang wrote:
>>> Shifting signed 32-bit value by 31 bits is undefined. Changing most
>>> significant bit to unsigned.
>>>
>>> Changes included in v2:
>>> - use subsystem specific subject lines
>>> - CC required mailing lists
>>>
>>> Signed-off-by: Jiunn Chang <[email protected]>
>>> ---
>>
>> Move version change lines here. They don't belong in the commit log.
>
> FWIW, in many cases people (maintainers) now *do* want them in the
> commit log. Here, they're just editorial, so agree, but if real
> technical changes were made due to reviews, they can indeed be useful.
>
> johannes
>

I went looking in the git log. Looks like there are several commits with
"Changes since" included in the commit log. It still appears to be
maintainer preference. Probably from networking and networking related
areas - wireless being one of them. This trend is recent it appears in
any case.

There is a value to seeing changes as the work evolves. However, there
is the concern that how log should it be.

This example commit has history from RFC stage and no doubt very useful
since this is a new driver.

8ef988b914bd449458eb2174febb67b0f137b33c

If we make this more of a norm, we do want to make sure, we evolve
from informal nature of these "Changes since", to "Commit log" text.

thanks,
-- Shuah

2019-07-04 18:50:06

by Jiunn Chang

[permalink] [raw]
Subject: Re: [Linux-kernel-mentees][PATCH v2] nl80211: Fix undefined behavior in bit shift

On Fri, Jun 28, 2019 at 03:57:05PM +0200, Johannes Berg wrote:
> On Wed, 2019-06-26 at 21:34 -0600, Shuah Khan wrote:
> > On 6/26/19 9:25 PM, Jiunn Chang wrote:
> > > Shifting signed 32-bit value by 31 bits is undefined. Changing most
> > > significant bit to unsigned.
> > >
> > > Changes included in v2:
> > > - use subsystem specific subject lines
> > > - CC required mailing lists
> > >
> > > Signed-off-by: Jiunn Chang <[email protected]>
> > > ---
> >
> > Move version change lines here. They don't belong in the commit log.
>
> FWIW, in many cases people (maintainers) now *do* want them in the
> commit log. Here, they're just editorial, so agree, but if real
> technical changes were made due to reviews, they can indeed be useful.
>
> johannes
>

Hello Johannes,

Would you like me to send v3 with the change log in the patch description?

THX,

Jiunn

2019-07-04 20:24:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [Linux-kernel-mentees][PATCH v2] nl80211: Fix undefined behavior in bit shift

On Thu, 2019-07-04 at 13:34 -0500, Jiunn Chang wrote:
> Would you like me to send v3 with the change log in the patch description?
>
No no, like I said, it's not very useful in this case anyway.

johannes