[src/trunk]: src/sys/dev/ic Avoid undefined behavior of signedness bit shift ...

[kamil <kamil%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/bfaf421ed9e1
branches:  trunk
changeset: 323951:bfaf421ed9e1
user:      kamil <kamil%NetBSD.org@localhost>
date:      Mon Jul 09 10:44:44 2018 +0000

description:
Avoid undefined behavior of signedness bit shift in ahcisata_core.c

sys/dev/ic/ahcisata_core.c:365:31, left shift of 1 by 31 places cannot be represented in type 'int'
sys/dev/ic/ahcisata_core.c:558:16, left shift of 1 by 31 places cannot be represented in type 'int'

Detected with Kernel Undefined Behavior Sanitizer.

This code could be refactored in future and switched to ISSET(9) API,
instead of reinventing the common functionality.

diffstat:

 sys/dev/ic/ahcisata_core.c |  64 +++++++++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 32 deletions(-)

diffs (216 lines):

diff -r 04d20d2536ca -r bfaf421ed9e1 sys/dev/ic/ahcisata_core.c
--- a/sys/dev/ic/ahcisata_core.c        Mon Jul 09 10:36:30 2018 +0000
+++ b/sys/dev/ic/ahcisata_core.c        Mon Jul 09 10:44:44 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ahcisata_core.c,v 1.61 2018/07/08 17:58:26 jdolecek Exp $      */
+/*     $NetBSD: ahcisata_core.c,v 1.62 2018/07/09 10:44:44 kamil Exp $ */
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.61 2018/07/08 17:58:26 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahcisata_core.c,v 1.62 2018/07/09 10:44:44 kamil Exp $");
 
 #include <sys/types.h>
 #include <sys/malloc.h>
@@ -175,7 +175,7 @@
        int i, port;
        
        for (i = 0, port = 0; i < AHCI_MAX_PORTS; i++) {
-               if ((sc->sc_ahci_ports & (1 << i)) == 0)
+               if ((sc->sc_ahci_ports & (1U << i)) == 0)
                        continue;
                if (port >= sc->sc_atac.atac_nchannels) {
                        aprint_error("%s: more ports than announced\n",
@@ -194,7 +194,7 @@
        struct ata_channel *chp;
 
        for (i = 0, port = 0; i < AHCI_MAX_PORTS; i++) {
-               if ((sc->sc_ahci_ports & (1 << i)) == 0)
+               if ((sc->sc_ahci_ports & (1U << i)) == 0)
                        continue;
                if (port >= sc->sc_atac.atac_nchannels) {
                        aprint_error("%s: more ports than announced\n",
@@ -362,7 +362,7 @@
                    DEBUG_PROBE);
        }
        for (i = 0, port = 0; i < AHCI_MAX_PORTS; i++) {
-               if ((sc->sc_ahci_ports & (1 << i)) == 0)
+               if ((sc->sc_ahci_ports & (1U << i)) == 0)
                        continue;
                if (port >= sc->sc_atac.atac_nchannels) {
                        aprint_error("%s: more ports than announced\n",
@@ -493,7 +493,7 @@
                achp = &sc->sc_channels[i];
                chp = &achp->ata_channel;
 
-               if ((sc->sc_ahci_ports & (1 << i)) == 0)
+               if ((sc->sc_ahci_ports & (1U << i)) == 0)
                        continue;
                if (i >= sc->sc_atac.atac_nchannels) {
                        aprint_error("%s: more ports than announced\n",
@@ -555,7 +555,7 @@
                r = 1;
                AHCI_WRITE(sc, AHCI_IS, is);
                for (i = 0; i < AHCI_MAX_PORTS; i++)
-                       if (is & (1 << i))
+                       if (is & (1U << i))
                                ahci_intr_port(sc, &sc->sc_channels[i]);
        }
        return r;
@@ -723,9 +723,9 @@
        AHCI_CMDH_SYNC(sc, achp, slot,
            BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
        /* start command */
-       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1 << slot);
+       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1U << slot);
        for (i = 0; i < timeout; i++) {
-               if ((AHCI_READ(sc, AHCI_P_CI(chp->ch_channel)) & (1 << slot)) ==
+               if ((AHCI_READ(sc, AHCI_P_CI(chp->ch_channel)) & (1U << slot)) ==
                    0)
                        return 0;
                is = AHCI_READ(sc, AHCI_P_IS(chp->ch_channel));
@@ -1069,7 +1069,7 @@
            DEBUG_XFERS);
 
        ata_channel_lock_owned(chp);
-       KASSERT((achp->ahcic_cmds_active & (1 << slot)) == 0);
+       KASSERT((achp->ahcic_cmds_active & (1U << slot)) == 0);
 
        cmd_tbl = achp->ahcic_cmd_tbl[slot];
        AHCIDEBUG_PRINT(("%s port %d tbl %p\n", AHCINAME(sc), chp->ch_channel,
@@ -1102,9 +1102,9 @@
                    AHCI_READ(sc, AHCI_GHC) & ~AHCI_GHC_IE);
        }
        /* start command */
-       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1 << slot);
+       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1U << slot);
        /* and says we started this command */
-       achp->ahcic_cmds_active |= 1 << slot;
+       achp->ahcic_cmds_active |= 1U << slot;
 
        if ((ata_c->flags & AT_POLL) == 0) {
                callout_reset(&xfer->c_timo_callout, mstohz(ata_c->timeout),
@@ -1188,8 +1188,8 @@
        }
 
        if (deactivate) {
-               KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-               achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+               KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+               achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
                ata_deactivate_xfer(chp, xfer);
        }
 
@@ -1211,8 +1211,8 @@
        if (ata_waitdrain_xfer_check(chp, xfer))
                return 0;
 
-       KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-       achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+       KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+       achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
        ata_deactivate_xfer(chp, xfer);
 
        if (xfer->c_flags & C_TIMEOU) {
@@ -1356,11 +1356,11 @@
                    AHCI_READ(sc, AHCI_GHC) & ~AHCI_GHC_IE);
        }
        if (xfer->c_flags & C_NCQ)
-               AHCI_WRITE(sc, AHCI_P_SACT(chp->ch_channel), 1 << xfer->c_slot);
+               AHCI_WRITE(sc, AHCI_P_SACT(chp->ch_channel), 1U << xfer->c_slot);
        /* start command */
-       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1 << xfer->c_slot);
+       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1U << xfer->c_slot);
        /* and says we started this command */
-       achp->ahcic_cmds_active |= 1 << xfer->c_slot;
+       achp->ahcic_cmds_active |= 1U << xfer->c_slot;
 
        if ((xfer->c_flags & C_POLL) == 0) {
                callout_reset(&xfer->c_timo_callout, mstohz(ATA_DELAY),
@@ -1440,8 +1440,8 @@
        ata_bio->r_error = WDCE_ABRT;
 
        if (deactivate) {
-               KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-               achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+               KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+               achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
                ata_deactivate_xfer(chp, xfer);
        }
 
@@ -1462,8 +1462,8 @@
        if (ata_waitdrain_xfer_check(chp, xfer))
                return 0;
 
-       KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-       achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+       KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+       achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
        ata_deactivate_xfer(chp, xfer);
 
        if (xfer->c_flags & C_TIMEOU) {
@@ -1677,7 +1677,7 @@
        switch (error) {
        case 0:
                /* Error out the particular NCQ xfer, then requeue the others */
-               if ((achp->ahcic_cmds_active & (1 << eslot)) != 0) {
+               if ((achp->ahcic_cmds_active & (1U << eslot)) != 0) {
                        xfer = ata_queue_hwslot_to_xfer(chp, eslot);
                        xfer->c_flags |= C_RECOVERED;
                        xfer->c_intr(chp, xfer,
@@ -1691,7 +1691,7 @@
                 * the error.
                 */
                for (slot = 0; slot < sc->sc_ncmds; slot++) {
-                       if ((achp->ahcic_cmds_active & (1 << slot)) != 0) {
+                       if ((achp->ahcic_cmds_active & (1U << slot)) != 0) {
                                xfer = ata_queue_hwslot_to_xfer(chp, slot);
                                xfer->c_intr(chp, xfer, tfd);
                        }
@@ -1720,7 +1720,7 @@
 
        /* Requeue all unfinished commands for same drive as failed command */ 
        for (slot = 0; slot < sc->sc_ncmds; slot++) {
-               if ((achp->ahcic_cmds_active & (1 << slot)) == 0)
+               if ((achp->ahcic_cmds_active & (1U << slot)) == 0)
                        continue;
 
                xfer = ata_queue_hwslot_to_xfer(chp, slot);
@@ -1943,9 +1943,9 @@
                    AHCI_READ(sc, AHCI_GHC) & ~AHCI_GHC_IE);
        }
        /* start command */
-       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1 << xfer->c_slot);
+       AHCI_WRITE(sc, AHCI_P_CI(chp->ch_channel), 1U << xfer->c_slot);
        /* and says we started this command */
-       achp->ahcic_cmds_active |= 1 << xfer->c_slot;
+       achp->ahcic_cmds_active |= 1U << xfer->c_slot;
 
        if ((xfer->c_flags & C_POLL) == 0) {
                callout_reset(&xfer->c_timo_callout, mstohz(sc_xfer->timeout),
@@ -2006,8 +2006,8 @@
        if (ata_waitdrain_xfer_check(chp, xfer))
                return 0;
 
-       KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-       achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+       KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+       achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
        ata_deactivate_xfer(chp, xfer);
 
        if (xfer->c_flags & C_TIMEOU) {
@@ -2074,8 +2074,8 @@
        }
 
        if (deactivate) {
-               KASSERT((achp->ahcic_cmds_active & (1 << xfer->c_slot)) != 0);
-               achp->ahcic_cmds_active &= ~(1 << xfer->c_slot);
+               KASSERT((achp->ahcic_cmds_active & (1U << xfer->c_slot)) != 0);
+               achp->ahcic_cmds_active &= ~(1U << xfer->c_slot);
                ata_deactivate_xfer(chp, xfer);
        }