netdev - Re: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: guard fwlog code by CONFIG_DEBUG

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: <e94f188d-3578-447f-8815-6c2393f2f807@roeck-us.net>
Date: Wed, 15 Oct 2025 10:53:45 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Jacob Keller <jacob.e.keller@...el.com>,
 Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: guard fwlog code by
 CONFIG_DEBUG_FS

On 10/15/25 10:32, Jacob Keller wrote:
> 
> 
> On 10/14/2025 10:24 PM, Michal Swiatkowski wrote:
>> On Tue, Oct 14, 2025 at 04:41:43PM -0700, Jacob Keller wrote:
>>>
>>>
>>> On 10/14/2025 7:11 AM, Michal Swiatkowski wrote:
>>>> Building the ixgbe without CONFIG_DEBUG_FS leads to a build error. Fix
>>>> that by guarding fwlog code.
>>>>
>>>> Fixes: 641585bc978e ("ixgbe: fwlog support for e610")
>>>> Reported-by: Guenter Roeck <linux@...ck-us.net>
>>>> Closes: https://lore.kernel.org/lkml/f594c621-f9e1-49f2-af31-23fbcb176058@roeck-us.net/
>>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>>>> ---
>>>
>>> Hm. It probably is best to make this optional and not require debugfs
>>> via kconfig.
>>
>> Make sense
>>
>>>
>>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 2 ++
>>>>   drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 8 ++++++++
>>>>   2 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>>>> index c2f8189a0738..c5d76222df18 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>>>> @@ -3921,6 +3921,7 @@ static int ixgbe_read_pba_string_e610(struct ixgbe_hw *hw, u8 *pba_num,
>>>>   	return err;
>>>>   }
>>>>   
>>>> +#ifdef CONFIG_DEBUG_FS
>>>>   static int __fwlog_send_cmd(void *priv, struct libie_aq_desc *desc, void *buf,
>>>>   			    u16 size)
>>>>   {
>>>> @@ -3952,6 +3953,7 @@ void ixgbe_fwlog_deinit(struct ixgbe_hw *hw)
>>>>   
>>>>   	libie_fwlog_deinit(&hw->fwlog);
>>>>   }
>>>> +#endif /* CONFIG_DEBUG_FS */
>>>>   
>>>
>>> What does the fwlog module from libie do? Seems likely that it won't
>>> compile without CONFIG_DEBUG_FS either...
>>
>> Right, it shouldn't, because there is a dependency on fs/debugfs.
>> It is building on my env, but maybe I don't have it fully cleaned.
>> I wonder, because in ice there wasn't a check (or select) for
>> CONFIG_DEBUG_FS for fwlog code.
>>
>> Looks like LIBIE_FWLOG should select DEBUG_FS, right?
>> I will send v2 with that, if it is fine.
>>
>> Thanks
>>
> My only worry is that could lead to ice selecting LIBIE_FWLOG which then
> selects DEBUG_FS which then means we implicitly require DEBUG_FS regardless.
> 
> I don't quite remember the semantics of select and whether that would
> let you build a kernel without DEBUG_FS.. I think some systems would
> like to be able to disable DEBUG_FS as a way of limiting scope of
> available interfaces exposed by the kernel.
> 

If fwlog depends on debugfs, why not spell out that dependency and
provide dummy functions if it isn't enabled ? The Kconfig entries
selecting it could then be changed to
	select LIBIE_FWLOG if DEBUG_FS

Guenter