linux-kernel - Re: [PATCH v5 6/7] misc: bcm-vk: add Broadcom VK driver

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]

Message-ID: <60372b2f-c03d-6384-43a7-8b97413b6672@broadcom.com>
Date:   Tue, 12 May 2020 23:31:28 -0700
From:   Scott Branden <scott.branden@...adcom.com>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        David Brown <david.brown@...aro.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Shuah Khan <shuah@...nel.org>, bjorn.andersson@...aro.org,
        Shuah Khan <skhan@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-fsdevel@...r.kernel.org,
        BCM Kernel Feedback <bcm-kernel-feedback-list@...adcom.com>,
        Olof Johansson <olof@...om.net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Colin Ian King <colin.king@...onical.com>,
        Kees Cook <keescook@...omium.org>,
        Takashi Iwai <tiwai@...e.de>, linux-kselftest@...r.kernel.org,
        Andy Gross <agross@...nel.org>,
        Desmond Yan <desmond.yan@...adcom.com>,
        James Hu <james.hu@...adcom.com>
Subject: Re: [PATCH v5 6/7] misc: bcm-vk: add Broadcom VK driver

Hi Luis,

On 2020-05-12 5:38 p.m., Luis Chamberlain wrote:
> On Thu, May 07, 2020 at 05:27:38PM -0700, Scott Branden wrote:
>> +#if defined(CONFIG_REQ_FW_INTO_BUF_PRIV)
>> +
>> +#define KERNEL_PREAD_FLAG_PART	0x0001 /* Allow reading part of file */
>> +#define REQUEST_FIRMWARE_INTO_BUF request_firmware_into_buf_priv
>> +int request_firmware_into_buf_priv(const struct firmware **firmware_p,
>> +				   const char *name, struct device *device,
>> +				   void *buf, size_t size,
>> +				   size_t offset, unsigned int pread_flags);
>> +
>> +#else
>> +
>> +#define REQUEST_FIRMWARE_INTO_BUF request_firmware_into_buf
>> +
>> +#endif
>> +
>> +#endif
> Please clean this up, the code must reflect only the code upstream. No
> config stuff like this should be used on the driver. I had to stop my
> review here.
The CONFIG_ prefix shouldn't have been there as there is no Kconfig 
option to select this.
Would like to just change it to a normal define without CONFIG_ prefix 
instead?
This code is here to allow a limited version of the driver to run on 
older kernels which do not have the necessary partial file read support.
By having it in the upstream codebase we don't need to maintain an 
internal version of the driver.  User can take the upstream kernel 
module and compile it against an old version of the kernel by via the 
define.
>    Luis
Regards,
Scott