* [PATCH][next] net: dsa: mv88e6xxx: avoid unintended sign extension on a 16 bit shift
@ 2018-02-15 19:12 Colin King
2018-02-15 20:27 ` Andrew Lunn
0 siblings, 1 reply; 3+ messages in thread
From: Colin King @ 2018-02-15 19:12 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The shifting of timehi by 16 bits to the left will be promoted to
a 32 bit signed int and then sign-extended to an u64. If the top bit
of timehi is set then all then all the upper bits of ns end up as also
being set because of the sign-extension. Fix this by casting timehi
to u64 before the shift.
Detected by CoverityScan, CID#1465288 ("Unintended sign extension")
Fixes: c6fe0ad2c349 ("net: dsa: mv88e6xxx: add rx/tx timestamping support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index b251d534b70d..758e35fa69ab 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -321,7 +321,7 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip,
*/
for ( ; skb; skb = skb_dequeue(rxq)) {
if (mv88e6xxx_ts_valid(status) && seq_match(skb, seq_id)) {
- u64 ns = timehi << 16 | timelo;
+ u64 ns = (u64)timehi << 16 | timelo;
mutex_lock(&chip->reg_lock);
ns = timecounter_cyc2time(&chip->tstamp_tc, ns);
--
2.15.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][next] net: dsa: mv88e6xxx: avoid unintended sign extension on a 16 bit shift
2018-02-15 19:12 [PATCH][next] net: dsa: mv88e6xxx: avoid unintended sign extension on a 16 bit shift Colin King
@ 2018-02-15 20:27 ` Andrew Lunn
2018-02-16 15:53 ` Richard Cochran
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2018-02-15 20:27 UTC (permalink / raw)
To: richardcochran
Cc: Vivien Didelot, Florian Fainelli, netdev, kernel-janitors,
linux-kernel
> diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> index b251d534b70d..758e35fa69ab 100644
> --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> @@ -321,7 +321,7 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip,
> */
> for ( ; skb; skb = skb_dequeue(rxq)) {
> if (mv88e6xxx_ts_valid(status) && seq_match(skb, seq_id)) {
> - u64 ns = timehi << 16 | timelo;
> + u64 ns = (u64)timehi << 16 | timelo;
Hi Richard
Do you prefer this, or making timehi and timelo a u64?
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][next] net: dsa: mv88e6xxx: avoid unintended sign extension on a 16 bit shift
2018-02-15 20:27 ` Andrew Lunn
@ 2018-02-16 15:53 ` Richard Cochran
0 siblings, 0 replies; 3+ messages in thread
From: Richard Cochran @ 2018-02-16 15:53 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, Florian Fainelli, netdev, kernel-janitors,
linux-kernel
On Thu, Feb 15, 2018 at 09:27:57PM +0100, Andrew Lunn wrote:
> Do you prefer this, or making timehi and timelo a u64?
The latter.
While you are at it, please move the definition of 'ns' to the start
of the function.
Thanks,
Richard
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-16 15:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-15 19:12 [PATCH][next] net: dsa: mv88e6xxx: avoid unintended sign extension on a 16 bit shift Colin King
2018-02-15 20:27 ` Andrew Lunn
2018-02-16 15:53 ` Richard Cochran
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).