Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] autofs - fix slab out of bounds read in getname_kernel()
@ 2018-07-03  6:45 Ian Kent
  2018-07-05 23:58 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Kent @ 2018-07-03  6:45 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-fsdevel, Tomas Bortoli, autofs mailing list,
	Kernel Mailing List, Dmitry Vyukov

Initial patch contributed by Tomas Bortoli.

The autofs subsystem does not check that the "path" parameter is
present for all cases where it is required when it is passed in
via the "param" struct.

In particular it isn't checked for the AUTOFS_DEV_IOCTL_OPENMOUNT_CMD
ioctl command.

To solve it, modify validate_dev_ioctl() function to check that a
path has been provided for ioctl commands that require it.

Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Signed-off-by: Ian Kent <raven@themaw.net>
Reported-by: syzbot+60c837b428dc84e83a93@syzkaller.appspotmail.com
---
 fs/autofs/dev-ioctl.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c
index ea4ca1445ab7..86eafda4a652 100644
--- a/fs/autofs/dev-ioctl.c
+++ b/fs/autofs/dev-ioctl.c
@@ -135,6 +135,15 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param)
 				cmd);
 			goto out;
 		}
+	} else {
+		unsigned int inr = _IOC_NR(cmd);
+
+		if (inr == AUTOFS_DEV_IOCTL_OPENMOUNT_CMD ||
+		    inr == AUTOFS_DEV_IOCTL_REQUESTER_CMD ||
+		    inr == AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD) {
+			err = -EINVAL;
+			goto out;
+		}
 	}
 
 	err = 0;
@@ -271,7 +280,8 @@ static int autofs_dev_ioctl_openmount(struct file *fp,
 	dev_t devid;
 	int err, fd;
 
-	/* param->path has already been checked */
+	/* param->path has been checked in validate_dev_ioctl() */
+
 	if (!param->openmount.devid)
 		return -EINVAL;
 
@@ -433,10 +443,7 @@ static int autofs_dev_ioctl_requester(struct file *fp,
 	dev_t devid;
 	int err = -ENOENT;
 
-	if (param->size <= AUTOFS_DEV_IOCTL_SIZE) {
-		err = -EINVAL;
-		goto out;
-	}
+	/* param->path has been checked in validate_dev_ioctl() */
 
 	devid = sbi->sb->s_dev;
 
@@ -521,10 +528,7 @@ static int autofs_dev_ioctl_ismountpoint(struct file *fp,
 	unsigned int devid, magic;
 	int err = -ENOENT;
 
-	if (param->size <= AUTOFS_DEV_IOCTL_SIZE) {
-		err = -EINVAL;
-		goto out;
-	}
+	/* param->path has been checked in validate_dev_ioctl() */
 
 	name = param->path;
 	type = param->ismountpoint.in.type;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] autofs - fix slab out of bounds read in getname_kernel()
  2018-07-03  6:45 [PATCH] autofs - fix slab out of bounds read in getname_kernel() Ian Kent
@ 2018-07-05 23:58 ` Andrew Morton
  2018-07-06  1:08   ` Ian Kent
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2018-07-05 23:58 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-fsdevel, Tomas Bortoli, autofs mailing list,
	Kernel Mailing List, Dmitry Vyukov

On Tue, 03 Jul 2018 14:45:15 +0800 Ian Kent <raven@themaw.net> wrote:

> Initial patch contributed by Tomas Bortoli.
> 
> The autofs subsystem does not check that the "path" parameter is
> present for all cases where it is required when it is passed in
> via the "param" struct.
> 
> In particular it isn't checked for the AUTOFS_DEV_IOCTL_OPENMOUNT_CMD
> ioctl command.
> 
> To solve it, modify validate_dev_ioctl() function to check that a
> path has been provided for ioctl commands that require it.
> 
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reported-by: syzbot+60c837b428dc84e83a93@syzkaller.appspotmail.com

So who is the primary author of this?  I'm assuming "Tomas" from the
signoff order?

The way of signifying this is to put the other author's From: line right
at the top of changelog.

I assume this is a root-only bug, so we don't need to backport the fix?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] autofs - fix slab out of bounds read in getname_kernel()
  2018-07-05 23:58 ` Andrew Morton
@ 2018-07-06  1:08   ` Ian Kent
  0 siblings, 0 replies; 3+ messages in thread
From: Ian Kent @ 2018-07-06  1:08 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-fsdevel, Tomas Bortoli, autofs mailing list,
	Kernel Mailing List, Dmitry Vyukov

On Thu, 2018-07-05 at 16:58 -0700, Andrew Morton wrote:
> On Tue, 03 Jul 2018 14:45:15 +0800 Ian Kent <raven@themaw.net> wrote:
> 
> > Initial patch contributed by Tomas Bortoli.
> > 
> > The autofs subsystem does not check that the "path" parameter is
> > present for all cases where it is required when it is passed in
> > via the "param" struct.
> > 
> > In particular it isn't checked for the AUTOFS_DEV_IOCTL_OPENMOUNT_CMD
> > ioctl command.
> > 
> > To solve it, modify validate_dev_ioctl() function to check that a
> > path has been provided for ioctl commands that require it.
> > 
> > Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > Reported-by: syzbot+60c837b428dc84e83a93@syzkaller.appspotmail.com
> 
> So who is the primary author of this?  I'm assuming "Tomas" from the
> signoff order?

Yes, Tomas was the originator but I did change it somewhat so added
my Signed-off-by as well.

> 
> The way of signifying this is to put the other author's From: line right
> at the top of changelog.

Oh, ok, will do in future, although my changes were fairly significant.

IMHO it was a joint effort more or less equal by both of us.
Nevertheless I would have been happy to list Tomas as the From author.

> 
> I assume this is a root-only bug, so we don't need to backport the fix?

It is, so yes that's right, unless Tomas has other recommendations.

Ian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-03  6:45 [PATCH] autofs - fix slab out of bounds read in getname_kernel() Ian Kent
2018-07-05 23:58 ` Andrew Morton
2018-07-06  1:08   ` Ian Kent

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git