linux-kernel - Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up

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:   Sat, 20 Oct 2018 22:22:07 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     olaf@...fle.de, sthemmin@...rosoft.com,
        Greg KH <gregkh@...uxfoundation.org>, jasowang@...hat.com,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Stable@...r.kernel.org, Michael.H.Kelley@...rosoft.com,
        Robo Bot <apw@...onical.com>, devel@...uxdriverproject.org,
        vkuznets@...hat.com, haiyangz@...rosoft.com
Subject: Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused
 by incorrect clean-up

On Sat, Oct 20, 2018 at 04:42:07PM +0200, Miguel Ojeda wrote:
> +On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter <dan.carpenter@...cle.com> wrote:
> >
> > It's not common at all.  It should be wrapped in a macro and put into
> > compiler.h.
> >
> > But I hope it does become adopted.  It's better than randomly grepping
> > for non-standard comments.
> 
> Using an attribute is indeed better whenever possible. In C++17 it is
> an standard attribute and there have been proposals to include some of
> them for C as well since 2016 at least, e.g. the latest for
> fallthrough at:
> 
>   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf
> 
> I have taken a quick look into supporting it (typing it here to save
> it on the mailing list :-), and we have:
> 
>   * gcc >= 7.1 supports -Wimplicit-fallthrough with
> __attribute__((fallthrough)), the comment syntax and the C++
> [[fallthrough]] syntax.
>   * gcc < 7.1 complains about empty declarations (it does not know
> about attributes for null statements) and also
> -Wdeclaration-after-statement.

I'm not sure I understand about empty decalarations.  The idea is that
we would do:

	case 3:
		frob();
		__fall_through();
	case 4:
		frob();

#if GCC > 7.1
#define __fall_through() __attribute__((fallthrough))
#else
#define __fall_through()
#endif

So long as GCC and static analysis tools understand about the attribute
that's enought to catch the bugs.  No one cares what clang and icc do.
We would just disable the fall through warnings on those until they are
updated to support the fallthrough attribute.

We wouldn't update all the fall through comments until later, but going
forward people could use the __fall_through() macro if they want.

regards,
dan carpenter