Re: [PATCH] tomoyo: Don't check open/getattr permission on sockets.

From: Al Viro
Date: Tue Jun 18 2019 - 16:54:46 EST


On Sun, Jun 16, 2019 at 03:49:00PM +0900, Tetsuo Handa wrote:
> Hello, Al.
>
> Q1: Do you agree that we should fix TOMOYO side rather than SOCKET_I()->sk
> management.

You do realize that sockets are not unique in that respect, right?
All kinds of interesting stuff can be accessed via /proc/*/fd/*, and
it _can_ be closed under you. So I'd suggest checking how your code
copes with similar for pipes, FIFOs, epoll, etc., accessed that way...

We are _not_ going to be checking that in fs/open.c - the stuff found
via /proc/*/fd/* can have the associated file closed by the time
we get to calling ->open() and we won't know that until said call.

> Q2: Do you see any problem with using f->f_path.dentry->d_inode ?
> Do we need to use d_backing_inode() or d_inode() ?

Huh? What's wrong with file_inode(f), in the first place? And
just when can that be NULL, while we are at it?

> > static int tomoyo_inode_getattr(const struct path *path)
> > {
> > + /* It is not safe to call tomoyo_get_socket_name(). */
> > + if (path->dentry->d_inode && S_ISSOCK(path->dentry->d_inode->i_mode))
> > + return 0;

Can that be called for a negative?