linux-kernel - Re: [RFC PATCH 35/35] syscalls: do not call sys

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:   Tue, 13 Mar 2018 06:46:49 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     linux-kernel@...r.kernel.org, luto@...nel.org,
        torvalds@...ux-foundation.org, viro@...iv.linux.org.uk,
        akpm@...ux-foundation.org
Subject: Re: [RFC PATCH 35/35] syscalls: do not call sys_close() within the
 kernel


* Dominik Brodowski <linux@...inikbrodowski.net> wrote:

> On Mon, Mar 12, 2018 at 08:37:19AM +0100, Ingo Molnar wrote:
> > 
> > * Dominik Brodowski <linux@...inikbrodowski.net> wrote:
> > 
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -1200,7 +1200,7 @@ SYSCALL_DEFINE1(close, unsigned int, fd)
> > >  
> > >  	return retval;
> > >  }
> > > -EXPORT_SYMBOL(sys_close);
> > > +
> > >  
> > >  /*
> > 
> > Nit: this introduces a stray newline.
> > 
> > > --- a/include/linux/syscalls.h
> > > +++ b/include/linux/syscalls.h
> > > @@ -1042,4 +1042,10 @@ static inline long ksys_lchown(const char __user *filename, uid_t user,
> > >  	return do_fchownat(AT_FDCWD, filename, user, group,
> > >  			     AT_SYMLINK_NOFOLLOW);
> > >  }
> > > +
> > > +extern int __close_fd(struct files_struct *files, unsigned int fd);
> > > +static inline int ksys_close(unsigned int fd)
> > > +{
> > > +	return __close_fd(current->files, fd);
> > > +}
> > 
> > Would be nice to reuse that stray newline after the __close_fd() prototype for the 
> > canonical stylistic separation of declarations from definitions.
> > 
> > It would also be very nice to add a short comment before ksys_close() that 
> > explains how it differs from sys_close(). This should reduce the amount of 
> > cargo-cult copying of existing ksys_close()/sys_close() patterns.
> 
> Thanks, have done that:
> 
> 	/*
> 	 * In contrast to sys_close(), this stub does not check whether the syscall
> 	 * should or should not be restarted, but returns the raw error codes from
> 	 * __close_fd().
> 	 */
> 
> Is that ok?

Yeah, looks good to me!

Thanks,

	Ingo