All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@codeaurora.org>
To: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Kees Cook <keescook@chromium.org>,
	"David S. Miller" <davem@davemloft.net>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH wireless-drivers] mt76x0: Remove VLA usage
Date: Wed, 08 Aug 2018 12:46:54 +0300	[thread overview]
Message-ID: <871sb942cx.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <20180808092450.GA25772@redhat.com> (Stanislaw Gruszka's message of "Wed, 8 Aug 2018 11:24:52 +0200")

Stanislaw Gruszka <sgruszka@redhat.com> writes:

> On Tue, Aug 07, 2018 at 03:50:40PM -0700, Kees Cook wrote:
>> Even with "const" variables, the compiler will generate warnings about
>> VLA usage. In the quest to remove all VLAs from the kernel[1], this uses
>> a #define instead of a const to do the array sizing.
>> 
>> [1]
>> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>> 
>> Fixes: e87b5039511a ("mt76x0: eeprom files")
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> Please include this for the v4.19 merge window. The VLA was introduced
>> with the new source file (which I also note is missing a SPDX line), so
>
> I thought SPDX line is needed only if file has no license and eeprom.c
> file and other mt76x0 files have specified the license. Is SPDX still
> needed in that case ?
>  
>> +#define MT_MAP_READS	DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16)
>>  static int
>>  mt76x0_efuse_physical_size_check(struct mt76x0_dev *dev)
>>  {
>> -	const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
>> -	u8 data[map_reads * 16];
>
> Why this is variable length array? DIV_ROUND_UP can not be calculated
> at compile time? But if so, macro do not change the situation either.

The commit log mentioned:

  "Even with "const" variables, the compiler will generate warnings about
   VLA usage."

So I guess the compiler (gcc?) is just not smart enough in this case?

-- 
Kalle Valo

WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH wireless-drivers] mt76x0: Remove VLA usage
Date: Wed, 08 Aug 2018 12:46:54 +0300	[thread overview]
Message-ID: <871sb942cx.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <20180808092450.GA25772-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> (Stanislaw Gruszka's message of "Wed, 8 Aug 2018 11:24:52 +0200")

Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:

> On Tue, Aug 07, 2018 at 03:50:40PM -0700, Kees Cook wrote:
>> Even with "const" variables, the compiler will generate warnings about
>> VLA usage. In the quest to remove all VLAs from the kernel[1], this uses
>> a #define instead of a const to do the array sizing.
>> 
>> [1]
>> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
>> 
>> Fixes: e87b5039511a ("mt76x0: eeprom files")
>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> Please include this for the v4.19 merge window. The VLA was introduced
>> with the new source file (which I also note is missing a SPDX line), so
>
> I thought SPDX line is needed only if file has no license and eeprom.c
> file and other mt76x0 files have specified the license. Is SPDX still
> needed in that case ?
>  
>> +#define MT_MAP_READS	DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16)
>>  static int
>>  mt76x0_efuse_physical_size_check(struct mt76x0_dev *dev)
>>  {
>> -	const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
>> -	u8 data[map_reads * 16];
>
> Why this is variable length array? DIV_ROUND_UP can not be calculated
> at compile time? But if so, macro do not change the situation either.

The commit log mentioned:

  "Even with "const" variables, the compiler will generate warnings about
   VLA usage."

So I guess the compiler (gcc?) is just not smart enough in this case?

-- 
Kalle Valo

  reply	other threads:[~2018-08-08 12:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-07 22:50 [PATCH wireless-drivers] mt76x0: Remove VLA usage Kees Cook
2018-08-07 22:50 ` Kees Cook
2018-08-08  4:53 ` Kalle Valo
2018-08-08  9:24 ` Stanislaw Gruszka
2018-08-08  9:46   ` Kalle Valo [this message]
2018-08-08  9:46     ` Kalle Valo
2018-08-08 15:41     ` Kees Cook
2018-08-09 10:41       ` Stanislaw Gruszka
2018-08-09 10:51         ` Kalle Valo
2018-08-09 15:09 ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871sb942cx.fsf@kamboji.qca.qualcomm.com \
    --to=kvalo@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sgruszka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.