linux-kernel - Re: [PATCH v5 7/7] fbdev: Make registered

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: <8c84428c-2740-4046-74c9-298b854944d0@roeck-us.net>
Date:   Wed, 11 May 2022 10:17:30 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Sam Ravnborg <sam@...nborg.org>,
        Javier Martinez Canillas <javierm@...hat.com>
Cc:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-fbdev@...r.kernel.org,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Helge Deller <deller@....de>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        kernel test robot <lkp@...el.com>,
        Jens Frederich <jfrederich@...il.com>,
        Jon Nettleton <jon.nettleton@...il.com>,
        linux-staging@...ts.linux.dev,
        Daniel Vetter <daniel.vetter@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Matthew Wilcox <willy@...radead.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Zhen Lei <thunder.leizhen@...wei.com>,
        Alex Deucher <alexander.deucher@....com>,
        Xiyu Yang <xiyuyang19@...an.edu.cn>,
        Zheyu Ma <zheyuma97@...il.com>
Subject: Re: [PATCH v5 7/7] fbdev: Make registered_fb[] private to fbmem.c

On 5/11/22 10:00, Sam Ravnborg wrote:
> Hi Javier.
> 
> On Wed, May 11, 2022 at 01:32:30PM +0200, Javier Martinez Canillas wrote:
>> From: Daniel Vetter <daniel.vetter@...ll.ch>
>>
>> Well except when the olpc dcon fbdev driver is enabled, that thing
>> digs around in there in rather unfixable ways.
>>
>> Cc oldc_dcon maintainers as fyi.
> 
> Another way to fix this is to mark FB_OLPC_DCON and add a TODO entry to
> fix this. We are really not supposed to carry such special code around
> to keep staging working.
> 
> I know this may not be a popular viewpoint, but just look at the ugly
> workarounds required here.
> 
> 	Sam
> 
> 
>>
>> v2: I typoed the config name (0day)
>>
>> Cc: kernel test robot <lkp@...el.com>
>> Cc: Jens Frederich <jfrederich@...il.com>
>> Cc: Jon Nettleton <jon.nettleton@...il.com>
>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>> Cc: linux-staging@...ts.linux.dev
>> Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch>
>> Reviewed-by: Javier Martinez Canillas <javierm@...hat.com>
>> Cc: Daniel Vetter <daniel@...ll.ch>
>> Cc: Helge Deller <deller@....de>
>> Cc: Matthew Wilcox <willy@...radead.org>
>> Cc: Sam Ravnborg <sam@...nborg.org>
>> Cc: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
>> Cc: Zhen Lei <thunder.leizhen@...wei.com>
>> Cc: Alex Deucher <alexander.deucher@....com>
>> Cc: Xiyu Yang <xiyuyang19@...an.edu.cn>
>> Cc: linux-fbdev@...r.kernel.org
>> Cc: Zheyu Ma <zheyuma97@...il.com>
>> Cc: Guenter Roeck <linux@...ck-us.net>
>> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
>> ---
>>
>> (no changes since v1)
>>
>>   drivers/video/fbdev/core/fbmem.c | 8 ++++++--
>>   include/linux/fb.h               | 7 +++----
>>   2 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index 265efa189bcc..6cab5f4c1fb3 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -50,10 +50,14 @@
>>   static DEFINE_MUTEX(registration_lock);
>>   
>>   struct fb_info *registered_fb[FB_MAX] __read_mostly;
>> -EXPORT_SYMBOL(registered_fb);
>> -
>>   int num_registered_fb __read_mostly;
>> +#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
>> +EXPORT_SYMBOL(registered_fb);
>>   EXPORT_SYMBOL(num_registered_fb);
>> +#endif
> 
> It is stuff like this I refer to as "ugly" in the comment above.
> 

My "solution" for that kind of thing is to use a namespace,
such as

EXPORT_SYMBOL_NS(registered_fb, FB_OLPC_DCON);
EXPORT_SYMBOL_NS(num_registered_fb, FB_OLPC_DCON);

and import it from the offending code. That avoids ifdefs
while at the same time limiting the use of the symbols
to the expected scope. Of course that could be abused but
that abuse would be obvious.

Guenter