Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200626130525.389469-1-lee.jones@linaro.org> <20200626130525.389469-5-lee.jones@linaro.org>
In-Reply-To: <20200626130525.389469-5-lee.jones@linaro.org>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 26 Jun 2020 16:37:16 +0200
Message-ID: <CAK8P3a2mjN6J22vLZkDQH5ONA4HGkqg6O+h4YYCKeVY1RKa+rA@mail.gmail.com>
Subject: Re: [PATCH 04/10] misc: lkdtm: bugs: At least try to use popuated variable
To:     Lee Jones <lee.jones@linaro.org>
Cc:     gregkh <gregkh@linuxfoundation.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Kees Cook <keescook@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 26, 2020 at 3:05 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> The result may not be intereresting, but not using a set variable
> is bad form and causes W=1 kernel builds to complain.
>
> Fixes the following W=1 warning(s):
>
>  drivers/misc/lkdtm/bugs.c: In function ‘lkdtm_STACK_GUARD_PAGE_LEADING’:
>  drivers/misc/lkdtm/bugs.c:331:25: warning: variable ‘byte’ set but not used [-Wunused-but-set-variable]
>  331 | volatile unsigned char byte;
>  | ^~~~
>  drivers/misc/lkdtm/bugs.c: In function ‘lkdtm_STACK_GUARD_PAGE_TRAILING’:
>  drivers/misc/lkdtm/bugs.c:345:25: warning: variable ‘byte’ set but not used [-Wunused-but-set-variable]
>  345 | volatile unsigned char byte;
>  | ^~~~
>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I think a clearer way to address this would be to add a cast to void than
to actually use the variable.

Looking at the implementation, it also seems odd to use a 'const char *' as
the source and a 'volatile char' as the destination, I would have expected
the opposite (marking the source volatile to force the access),
though I suppose the effect is the same.

    Arnd