linux-kernel - Re: [PATCH 02/13] uapi: General notification ring definitions [ver #4]

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:   Sat, 8 Jun 2019 21:35:33 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     David Howells <dhowells@...hat.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     viro@...iv.linux.org.uk, raven@...maw.net,
        linux-fsdevel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-block@...r.kernel.org, keyrings@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/13] uapi: General notification ring definitions [ver
 #4]

On 6/7/19 8:51 AM, David Howells wrote:
> Darrick J. Wong <darrick.wong@...cle.com> wrote:
> 

>>> +	__u32			info;
>>> +#define WATCH_INFO_OVERRUN	0x00000001	/* Event(s) lost due to overrun */
>>> +#define WATCH_INFO_ENOMEM	0x00000002	/* Event(s) lost due to ENOMEM */
>>> +#define WATCH_INFO_RECURSIVE	0x00000004	/* Change was recursive */
>>> +#define WATCH_INFO_LENGTH	0x000001f8	/* Length of record / sizeof(watch_notification) */
>>
>> This is a mask, isn't it?  Could we perhaps have some helpers here?
>> Something along the lines of...
>>
>> #define WATCH_INFO_LENGTH_MASK	0x000001f8
>> #define WATCH_INFO_LENGTH_SHIFT	3
>>
>> static inline size_t watch_notification_length(struct watch_notification *wn)
>> {
>> 	return (wn->info & WATCH_INFO_LENGTH_MASK) >> WATCH_INFO_LENGTH_SHIFT *
>> 			sizeof(struct watch_notification);
>> }
>>
>> static inline struct watch_notification *watch_notification_next(
>> 		struct watch_notification *wn)
>> {
>> 	return wn + ((wn->info & WATCH_INFO_LENGTH_MASK) >>
>> 			WATCH_INFO_LENGTH_SHIFT);
>> }
> 
> No inline functions in UAPI headers, please.  I'd love to kill off the ones
> that we have, but that would break things.

Hi David,

What is the problem with inline functions in UAPI headers?

>> ...so that we don't have to opencode all of the ring buffer walking
>> magic and stuff?
> 
> There'll end up being a small userspace library, I think.

>>> +};
>>> +
>>> +#define WATCH_LENGTH_SHIFT	3
>>> +
>>> +struct watch_queue_buffer {
>>> +	union {
>>> +		/* The first few entries are special, containing the
>>> +		 * ring management variables.
>>
>> The first /two/ entries, correct?
> 
> Currently two.
> 
>> Also, weird multiline comment style.
> 
> Not really.

Yes really.

>>> +		 */

It does not match the preferred coding style for multi-line comments
according to coding-style.rst.


thanks.
-- 
~Randy