* [PATCH] ipv6: sr: fix useless rol32 call on hash
@ 2018-07-17 15:52 ` Colin King
0 siblings, 0 replies; 8+ messages in thread
From: Colin King @ 2018-07-17 15:52 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The rol32 call is currently rotating hash but the rol'd value is
being discarded. I believe the current code is incorrect and hash
should be assigned the rotated value returned from rol32.
Detected by CoverityScan, CID#1468411 ("Useless call")
Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/ipv6/seg6_iptunnel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 19ccf0dc996c..a8854dd3e9c5 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -101,7 +101,7 @@ static __be32 seg6_make_flowlabel(struct net *net, struct sk_buff *skb,
if (do_flowlabel > 0) {
hash = skb_get_hash(skb);
- rol32(hash, 16);
+ hash = rol32(hash, 16);
flowlabel = (__force __be32)hash & IPV6_FLOWLABEL_MASK;
} else if (!do_flowlabel && skb->protocol == htons(ETH_P_IPV6)) {
flowlabel = ip6_flowlabel(inner_hdr);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] ipv6: sr: fix useless rol32 call on hash
@ 2018-07-17 15:52 ` Colin King
0 siblings, 0 replies; 8+ messages in thread
From: Colin King @ 2018-07-17 15:52 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The rol32 call is currently rotating hash but the rol'd value is
being discarded. I believe the current code is incorrect and hash
should be assigned the rotated value returned from rol32.
Detected by CoverityScan, CID#1468411 ("Useless call")
Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/ipv6/seg6_iptunnel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 19ccf0dc996c..a8854dd3e9c5 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -101,7 +101,7 @@ static __be32 seg6_make_flowlabel(struct net *net, struct sk_buff *skb,
if (do_flowlabel > 0) {
hash = skb_get_hash(skb);
- rol32(hash, 16);
+ hash = rol32(hash, 16);
flowlabel = (__force __be32)hash & IPV6_FLOWLABEL_MASK;
} else if (!do_flowlabel && skb->protocol = htons(ETH_P_IPV6)) {
flowlabel = ip6_flowlabel(inner_hdr);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
2018-07-17 15:52 ` Colin King
@ 2018-07-17 16:03 ` David Lebrun
-1 siblings, 0 replies; 8+ messages in thread
From: David Lebrun @ 2018-07-17 16:03 UTC (permalink / raw)
To: Colin King, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
On 07/17/2018 04:52 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> The rol32 call is currently rotating hash but the rol'd value is
> being discarded. I believe the current code is incorrect and hash
> should be assigned the rotated value returned from rol32.
>
> Detected by CoverityScan, CID#1468411 ("Useless call")
>
> Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
Acked-by: dlebrun@google.com
Good catch, thanks !
In that case, the same issue is present in
include/net/ipv6.h:ip6_make_flowlabel().
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
@ 2018-07-17 16:03 ` David Lebrun
0 siblings, 0 replies; 8+ messages in thread
From: David Lebrun @ 2018-07-17 16:03 UTC (permalink / raw)
To: Colin King, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
On 07/17/2018 04:52 PM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> The rol32 call is currently rotating hash but the rol'd value is
> being discarded. I believe the current code is incorrect and hash
> should be assigned the rotated value returned from rol32.
>
> Detected by CoverityScan, CID#1468411 ("Useless call")
>
> Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
Acked-by: dlebrun@google.com
Good catch, thanks !
In that case, the same issue is present in
include/net/ipv6.h:ip6_make_flowlabel().
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
2018-07-17 16:03 ` David Lebrun
@ 2018-07-17 17:08 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2018-07-17 17:08 UTC (permalink / raw)
To: David Lebrun, Colin King, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
On Tue, 2018-07-17 at 17:03 +0100, David Lebrun wrote:
> On 07/17/2018 04:52 PM, Colin King wrote:
> > From: Colin Ian King<colin.king@canonical.com>
> >
> > The rol32 call is currently rotating hash but the rol'd value is
> > being discarded. I believe the current code is incorrect and hash
> > should be assigned the rotated value returned from rol32.
> >
> > Detected by CoverityScan, CID#1468411 ("Useless call")
> >
> > Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> > Signed-off-by: Colin Ian King<colin.king@canonical.com>
>
> Acked-by: dlebrun@google.com
>
> Good catch, thanks !
>
> In that case, the same issue is present in
> include/net/ipv6.h:ip6_make_flowlabel().
Perhaps all of the ror and rol definitions should add
__must_check
Something like the below and perhaps many more of the
functions that return some value should have __must_chedk
added as well.
---
include/linux/bitops.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index af419012d77d..3cddde65c8bb 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -67,7 +67,7 @@ static inline __u64 rol64(__u64 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u64 ror64(__u64 word, unsigned int shift)
+static inline __must_check __u64 ror64(__u64 word, unsigned int shift)
{
return (word >> shift) | (word << (64 - shift));
}
@@ -77,7 +77,7 @@ static inline __u64 ror64(__u64 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u32 rol32(__u32 word, unsigned int shift)
+static inline __must_check __u32 rol32(__u32 word, unsigned int shift)
{
return (word << shift) | (word >> ((-shift) & 31));
}
@@ -87,7 +87,7 @@ static inline __u32 rol32(__u32 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u32 ror32(__u32 word, unsigned int shift)
+static inline __must_check __u32 ror32(__u32 word, unsigned int shift)
{
return (word >> shift) | (word << (32 - shift));
}
@@ -97,7 +97,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u16 rol16(__u16 word, unsigned int shift)
+static inline __must_check __u16 rol16(__u16 word, unsigned int shift)
{
return (word << shift) | (word >> (16 - shift));
}
@@ -107,7 +107,7 @@ static inline __u16 rol16(__u16 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u16 ror16(__u16 word, unsigned int shift)
+static inline __must_check __u16 ror16(__u16 word, unsigned int shift)
{
return (word >> shift) | (word << (16 - shift));
}
@@ -117,7 +117,7 @@ static inline __u16 ror16(__u16 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u8 rol8(__u8 word, unsigned int shift)
+static inline __must_check __u8 rol8(__u8 word, unsigned int shift)
{
return (word << shift) | (word >> (8 - shift));
}
@@ -127,7 +127,7 @@ static inline __u8 rol8(__u8 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u8 ror8(__u8 word, unsigned int shift)
+static inline __must_check __u8 ror8(__u8 word, unsigned int shift)
{
return (word >> shift) | (word << (8 - shift));
}
@@ -139,7 +139,7 @@ static inline __u8 ror8(__u8 word, unsigned int shift)
*
* This is safe to use for 16- and 8-bit types as well.
*/
-static inline __s32 sign_extend32(__u32 value, int index)
+static inline __must_check __s32 sign_extend32(__u32 value, int index)
{
__u8 shift = 31 - index;
return (__s32)(value << shift) >> shift;
@@ -150,7 +150,7 @@ static inline __s32 sign_extend32(__u32 value, int index)
* @value: value to sign extend
* @index: 0 based bit index (0<=index<64) to sign bit
*/
-static inline __s64 sign_extend64(__u64 value, int index)
+static inline __must_check __s64 sign_extend64(__u64 value, int index)
{
__u8 shift = 63 - index;
return (__s64)(value << shift) >> shift;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
@ 2018-07-17 17:08 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2018-07-17 17:08 UTC (permalink / raw)
To: David Lebrun, Colin King, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev
Cc: kernel-janitors, linux-kernel
On Tue, 2018-07-17 at 17:03 +0100, David Lebrun wrote:
> On 07/17/2018 04:52 PM, Colin King wrote:
> > From: Colin Ian King<colin.king@canonical.com>
> >
> > The rol32 call is currently rotating hash but the rol'd value is
> > being discarded. I believe the current code is incorrect and hash
> > should be assigned the rotated value returned from rol32.
> >
> > Detected by CoverityScan, CID#1468411 ("Useless call")
> >
> > Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> > Signed-off-by: Colin Ian King<colin.king@canonical.com>
>
> Acked-by: dlebrun@google.com
>
> Good catch, thanks !
>
> In that case, the same issue is present in
> include/net/ipv6.h:ip6_make_flowlabel().
Perhaps all of the ror and rol definitions should add
__must_check
Something like the below and perhaps many more of the
functions that return some value should have __must_chedk
added as well.
---
include/linux/bitops.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index af419012d77d..3cddde65c8bb 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -67,7 +67,7 @@ static inline __u64 rol64(__u64 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u64 ror64(__u64 word, unsigned int shift)
+static inline __must_check __u64 ror64(__u64 word, unsigned int shift)
{
return (word >> shift) | (word << (64 - shift));
}
@@ -77,7 +77,7 @@ static inline __u64 ror64(__u64 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u32 rol32(__u32 word, unsigned int shift)
+static inline __must_check __u32 rol32(__u32 word, unsigned int shift)
{
return (word << shift) | (word >> ((-shift) & 31));
}
@@ -87,7 +87,7 @@ static inline __u32 rol32(__u32 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u32 ror32(__u32 word, unsigned int shift)
+static inline __must_check __u32 ror32(__u32 word, unsigned int shift)
{
return (word >> shift) | (word << (32 - shift));
}
@@ -97,7 +97,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u16 rol16(__u16 word, unsigned int shift)
+static inline __must_check __u16 rol16(__u16 word, unsigned int shift)
{
return (word << shift) | (word >> (16 - shift));
}
@@ -107,7 +107,7 @@ static inline __u16 rol16(__u16 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u16 ror16(__u16 word, unsigned int shift)
+static inline __must_check __u16 ror16(__u16 word, unsigned int shift)
{
return (word >> shift) | (word << (16 - shift));
}
@@ -117,7 +117,7 @@ static inline __u16 ror16(__u16 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u8 rol8(__u8 word, unsigned int shift)
+static inline __must_check __u8 rol8(__u8 word, unsigned int shift)
{
return (word << shift) | (word >> (8 - shift));
}
@@ -127,7 +127,7 @@ static inline __u8 rol8(__u8 word, unsigned int shift)
* @word: value to rotate
* @shift: bits to roll
*/
-static inline __u8 ror8(__u8 word, unsigned int shift)
+static inline __must_check __u8 ror8(__u8 word, unsigned int shift)
{
return (word >> shift) | (word << (8 - shift));
}
@@ -139,7 +139,7 @@ static inline __u8 ror8(__u8 word, unsigned int shift)
*
* This is safe to use for 16- and 8-bit types as well.
*/
-static inline __s32 sign_extend32(__u32 value, int index)
+static inline __must_check __s32 sign_extend32(__u32 value, int index)
{
__u8 shift = 31 - index;
return (__s32)(value << shift) >> shift;
@@ -150,7 +150,7 @@ static inline __s32 sign_extend32(__u32 value, int index)
* @value: value to sign extend
* @index: 0 based bit index (0<=index<64) to sign bit
*/
-static inline __s64 sign_extend64(__u64 value, int index)
+static inline __must_check __s64 sign_extend64(__u64 value, int index)
{
__u8 shift = 63 - index;
return (__s64)(value << shift) >> shift;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
2018-07-17 15:52 ` Colin King
@ 2018-07-18 22:11 ` David Miller
-1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-07-18 22:11 UTC (permalink / raw)
To: colin.king; +Cc: kuznet, yoshfuji, netdev, kernel-janitors, linux-kernel
From: Colin King <colin.king@canonical.com>
Date: Tue, 17 Jul 2018 16:52:54 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The rol32 call is currently rotating hash but the rol'd value is
> being discarded. I believe the current code is incorrect and hash
> should be assigned the rotated value returned from rol32.
>
> Detected by CoverityScan, CID#1468411 ("Useless call")
>
> Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: fix useless rol32 call on hash
@ 2018-07-18 22:11 ` David Miller
0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-07-18 22:11 UTC (permalink / raw)
To: colin.king; +Cc: kuznet, yoshfuji, netdev, kernel-janitors, linux-kernel
From: Colin King <colin.king@canonical.com>
Date: Tue, 17 Jul 2018 16:52:54 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The rol32 call is currently rotating hash but the rol'd value is
> being discarded. I believe the current code is incorrect and hash
> should be assigned the rotated value returned from rol32.
>
> Detected by CoverityScan, CID#1468411 ("Useless call")
>
> Fixes: b5facfdba14c ("ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-18 22:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17 15:52 [PATCH] ipv6: sr: fix useless rol32 call on hash Colin King
2018-07-17 15:52 ` Colin King
2018-07-17 16:03 ` David Lebrun
2018-07-17 16:03 ` David Lebrun
2018-07-17 17:08 ` Joe Perches
2018-07-17 17:08 ` Joe Perches
2018-07-18 22:11 ` David Miller
2018-07-18 22:11 ` David Miller
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.