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: <f5d50007-5b48-47cb-8133-72fca274d562@gmx.de>
Date: Tue, 9 Dec 2025 09:22:49 +0100
From: Helge Deller <deller@....de>
To: Thomas Zimmermann <tzimmermann@...e.de>,
 Chintan Patel <chintanlike@...il.com>, linux-fbdev@...r.kernel.org,
 linux-staging@...ts.linux.dev, linux-omap@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 andy@...nel.org, gregkh@...uxfoundation.org
Subject: Re: [PATCH 0/3] fbdev: Guard sysfs interfaces under CONFIG_FB_DEVICE

On 12/9/25 08:27, Thomas Zimmermann wrote:
> Hi
> 
> Am 09.12.25 um 05:27 schrieb Chintan Patel:
>> Hi all,
>>
>> This small series makes several legacy fbdev drivers buildable with
>> CONFIG_FB_DEVICE=n. Currently, multiple fbdev drivers rely on fb_info->dev
>> and sysfs attribute registration unconditionally, which leads to build
>> failures whenever FB_DEVICE is disabled.
>>
>> Thomas previously noted that FB_DEVICE should eventually become optional
>> and that drivers should not depend on sysfs or fb_info->dev being present
>> unless the Kconfig explicitly selects it. This series pushes in that
>> direction by tightening the FB_DEVICE dependency boundary without changing
>> any runtime behaviour when FB_DEVICE=y.
>>
>> What this series does *not* change
>>
>> - No functional behaviour changes when FB_DEVICE=y.
>> - No removal of sysfs interfaces.
>> - No changes to fbops, memory allocation, or display update paths.
>>
>> Build & test coverage
>>
>> Tested with the following combinations:
>>
>> 1. **FB=y, FB_DEVICE=y**
>>     - Baseline configuration; no regressions expected.
>>
>> 2. **FB=y, FB_DEVICE=n**
>>     - Drivers build successfully.
>>     - No sysfs attributes are created.
>>     - fbdev devices operate normally (where applicable).
>>
>> 3. **FB=n**
>>     - Drivers depend on FB, so they properly do not build, unchanged.
>>
>> Motivation
>>
>> This moves fbdev closer to supporting FB_DEVICE as truly optional, helps
>> reduce Kconfig entanglement, and clears several long-standing TODO items
>> as suggested by Thomas Zimmermann around legacy sysfs usage inside fbdev
>> drivers.
>>
>> Feedback is welcome, especially on whether the guard boundaries around
>> sysfs are placed correctly or whether more logic should be pulled under
>> CONFIG_FB_DEVICE.
> 
> I left a comment on the first patch. If things still build nicely, then
> 
> Acked-by: Thomas Zimmermann <tzimmermann@...e.de>
> 
> for the series.

This whole series adds a whole lot of ifdef'ery, which I think is the
worst approach. It makes the code less readable and leads to two code
paths, which may trigger different build errors depending on the config.

I'm sure it must be possible to do the same without adding more #ifdefs,
e.g. by introducing a function like   dev_of_fbinfo(fbinfo)  which
simply returns NULL for the FB_DEVICE=n case.  Then, that value can be tested
like
	if (dev_of_fbinfo(fbinfo))
		{...do-the-things...}
For the FB_DEVICE=n case this will then be optimized out by the compiler,
while you still have full compiler syntax checking.

Thoughts?

Helge

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ