linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Ian Kent <raven@themaw.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	autofs mailing list <autofs@vger.kernel.org>,
	Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] autofs - fix autofs_sbi() does not check super block type
Date: Mon, 27 Aug 2018 02:03:58 +0100	[thread overview]
Message-ID: <20180827010358.GZ6515@ZenIV.linux.org.uk> (raw)
In-Reply-To: <153475422934.17131.7563724552005298277.stgit@pluto.themaw.net>

On Mon, Aug 20, 2018 at 04:37:09PM +0800, Ian Kent wrote:
> The autofs_sbi() inline function does not check the super block
> magic number to verify it has been given an autofs super block.

IMO it's the wrong way to fix it.  The one and only caller where that
check might trigger is

                if (!fp) {
                        if (cmd == AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD)
                                goto cont;
                        err = -EBADF;
                        goto out;
                }

                sbi = autofs_dev_ioctl_sbi(fp);
                if (!sbi || sbi->magic != AUTOFS_SBI_MAGIC) {
                        err = -EINVAL;
                        fput(fp);
                        goto out;
                }
with
static struct autofs_sb_info *autofs_dev_ioctl_sbi(struct file *f)
{
        struct autofs_sb_info *sbi = NULL;
        struct inode *inode;

        if (f) { 
                inode = file_inode(f);
                sbi = autofs_sbi(inode->i_sb);
        }
        return sbi;
}

First of all, what is that `if (f)' doing in there?  We have just checked
that in the only caller.

Next, dereferencing the result of autofs_sbi() does need to be preceded
by making sure that superblock is autofs one, all right... and what are
we doing in that first dereferencing, again?

IOW, turn that into

	if (!fp) {
		....
		goto out;
	}
	sb = file_inode(fp)->i_sb;
	if (sb->s_type != &autofs_fs_type)
		bugger off
	sbi = autofs_sbi(sb);
	....

and be done with that.  Other callers of autofs_sbi() really shouldn't
happen to other filesystem's superblocks...

  reply	other threads:[~2018-08-27  4:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20  8:37 [PATCH] autofs - fix autofs_sbi() does not check super block type Ian Kent
2018-08-27  1:03 ` Al Viro [this message]
2018-08-29  6:35   ` Ian Kent

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180827010358.GZ6515@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=autofs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raven@themaw.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).