Re: Kernel crash in free_pipe_info()

[Linus Torvalds]

On Mon, Oct 30, 2017 at 7:08 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> I'm not seeing anything that makes sense. I'll have to think about this.

Al, would you mind taking a look at the error handling in create_pipe_files().

In particular, look here:

 - we start out allocating the inode with "get_pipe_inode().

   That sets up a inode->i_pipe, with pipe->files initialized to 2.
Fine. We're going to have two file descriptors.

 - we then create the dummy path:

        path.dentry = d_alloc_pseudo(pipe_mnt->mnt_sb, &empty_name);

   fine fine. Again, this looks all good for the success cases.

But the *error* cases are a bit dodgy, aren't they?

We have three different error cases:

 - we couldn't even allocate a dentry. We do

        free_pipe_info(inode->i_pipe);
        iput(inode);

 - we couldn't allocate any file at all:

        free_pipe_info(inode->i_pipe);
        path_put(&path);

 - we allocated the first file, but not the second:

        put_filp(f);
        free_pipe_info(inode->i_pipe);
        path_put(&path);

and it worries me a bit that in all those error cases, we end up doing
that "free_pipe_info()", but we basically do this half-arsed job of
freeing things.

For example, we use "put_filp()" to free the file pointer, not "fput()".

We do that "free_pipe_info(inode->i_pipe);", but we never actually
clear inode->i_pipe, so now we have an inode that looks like a pipe
inode, and has a stale pointer to a pipe_inode_info.

It all looks technically correct. It's fine to use put_filp(), because
the file pointer has never really been used. And the inode should
never get re-used anyway without going through the whole reinit in
inode_init_always().

So I don't see anything *wrong*, but I see a lot that is just unusual,
and seems to depend on half-initialized state being fine. Can you look
at this?

                Linus