linux-kernel - Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

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]

Date:   Wed, 17 Feb 2021 16:25:49 +0000
From:   Chris Down <chris@...isdown.name>
To:     Petr Mladek <pmladek@...e.com>
Cc:     linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        John Ogness <john.ogness@...utronix.de>,
        Johannes Weiner <hannes@...xchg.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>, kernel-team@...com
Subject: Re: code style: Re: [PATCH v4] printk: Userspace format enumeration
 support

Petr Mladek writes:
>What about storing the pointer to struct pf_object into
>struct printk_fmt_sec *ps into the s->file->f_inode->i_private?
>Then we would not need any global list/table at all.

Unless I'm misreading the debugfs code, I think the following is possible:

open(f);
   debugfs_file_get(f);
   fops->open();
     inode->private = ps;
   debugfs_file_put(f);

remove_printk_fmt_sec(); /* kfree ps */

read(f);
   debugfs_file_get(f);
   fops->read();
     ps = inode->private;  /* invalid */
   debugfs_file_put(f);

That's the reason why the code looks up from the module address again during 
_read. Maybe I'm missing something? :-)

>> Oh, I meant to change the name for v4 but neglected to do so. Sounds good,
>> will do.
>
>Thanks a lot. I am sorry that I ask you to do so many changes.
>I talked about the style early enough to make the review easy.
>Also I think that it is not ideal and annoing to do these
>mass changes and refactoring when the code is already reviewed,
>tested, and functional.

Quite the opposite: thanks a lot for taking so much time to provide valuable 
feedback :-) As someone who mostly works on mm code, having you to provide 
feedback as printk maintainer is really helpful. Even if we disagree on some 
stuff, it's really important that we have a good shared understanding of what 
we eventually agree upon.