linux-kernel - Re: [sgi-xp] Missing break or false positive?

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]

Date:   Wed, 20 Mar 2019 09:37:33 -0500
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Cliff Whickman <cpw@....com>, Robin Holt <robinmholt@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, Kees Cook <keescook@...omium.org>
Subject: Re: [sgi-xp] Missing break or false positive?

Hi all,

Friendly ping:

Who can provide some feedback on this?

Thanks
--
Gustavo

On 2/26/19 10:24 PM, Gustavo A. R. Silva wrote:
> Hi all,
> 
> I'm taking a look into the following piece of code in drivers/misc/sgi-xp/xpc_uv.c:
> 
>  561         case XPC_ACTIVATE_MQ_MSG_CHCTL_OPENCOMPLETE_UV: {
>  562                 struct xpc_activate_mq_msg_chctl_opencomplete_uv *msg;
>  563 
>  564                 if (!part_setup)
>  565                         break;
>  566 
>  567                 msg = container_of(msg_hdr, struct
>  568                                 xpc_activate_mq_msg_chctl_opencomplete_uv, hdr);
>  569                 spin_lock_irqsave(&part->chctl_lock, irq_flags);
>  570                 part->chctl.flags[msg->ch_number] |= XPC_CHCTL_OPENCOMPLETE;
>  571                 spin_unlock_irqrestore(&part->chctl_lock, irq_flags);
>  572 
>  573                 xpc_wakeup_channel_mgr(part);
>  574         }
> 
> and I'm trying to figure out if the following warning is due to a missing break
> at the end of the case, or if this is just a false positive and a /* fall through */
> annotation should be added:
> 
> drivers/misc/sgi-xp/xpc_uv.c: In function ‘xpc_handle_activate_mq_msg_uv’:
> drivers/misc/sgi-xp/xpc_uv.c:573:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    xpc_wakeup_channel_mgr(part);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/misc/sgi-xp/xpc_uv.c:575:2: note: here
>   case XPC_ACTIVATE_MQ_MSG_MARK_ENGAGED_UV:
>   ^~~~
> 
> The piece of code above was introduced by the following commit in 2009:
> 
> efdd06ed181a88a11e612238c1ac04668e665395
> 
> The cases are pretty similar, and the fact that this code was introduced
> in the middle of the switch statement and not at the end or the beginning,
> leads me to believe that this is a false positive.  On the other hand,
> all the other cases end with a break or a return but this one.  So, I
> better ask your opinions about this.
> 
> Thanks
> --
> Gustavo
>