Re: [RFC][PATCH 01/42] drm_mode_create_lease_ioctl(): fix open-coded filp_clone_open()

[Al Viro]

On Wed, Jul 11, 2018 at 04:25:55PM +0100, Al Viro wrote:

> FWIW, looking at the ->f_flags handling, I'm seriously tempted to do
> alloc_file_pseudo(inode, mnt, name, f_flags, ops).
> 
> Reason: right now all but two callers of alloc_file_pseudo() are followed
> by setting ->f_flags and for all those callers the mode argument passed
> to alloc_file_pseudo() is equal to OPEN_FMODE(->f_flags value to be).
> 
> The exceptions are __shmem_file_setup() and hugetlb_file_setup().  Both
> end up with FMODE_READ | FMODE_WRITE combined with 0 (i.e. O_RDONLY) in
> f_flags.  Which smells like a bug in making, at the very least.
> 
> Unless somebody has a good reason why those shouldn't have O_RDWR,
> I want to add (and fold back into individual "convert to alloc_file_pseudo"
> commits) the following:

[snip]

> Objections?  Another odd beastie is do_shmat() - there we end up with
> f_mode not matching f_flags, same way as in shmem and hugetlb.  If we
> could rectify that one as well, we'd be able to switch alloc_file_clone()
> to flags as well.  I would obviously prefer that kind of change to happen
> before these helpers go into mainline...

Actually, looking at the entire thing, I'm rather tempted to go for
	alloc_empty_file(f_flags, cred)
setting both f_flags and f_flags-derived part of f_mode, making
	alloc_file_pseudo(inode, mnt, name, f_flags, ops)
	alloc_file_clone(base, f_flags, ops)
do the same automatically as they call alloc_empty_file().

do_dentry_open() would, instead of
        f->f_mode |= OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
                                FMODE_PREAD | FMODE_PWRITE;
do just
        f->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
just before calling ->open(), with comment along the lines of
"usually we want those set; let ->open() remove the wrong ones
if it wants to".