zfsdev_getminor() should check for invalid file handles

Unit testing at ClusterHQ found that passing an invalid file handle to
zfs_ioc_hold results in a NULL pointer dereference on a system without
assertions:

IP: [<ffffffffa0218aa0>] zfsdev_getminor+0x10/0x20 [zfs]
Call Trace:
[<ffffffffa021b4b0>] zfs_onexit_fd_hold+0x20/0x40 [zfs]
[<ffffffffa0214043>] zfs_ioc_hold+0x93/0xd0 [zfs]
[<ffffffffa0215890>] zfsdev_ioctl+0x200/0x500 [zfs]

An assertion would have caught this had they been enabled, but this is
something that the kernel module should handle without failing.  We
resolve this by searching the linked list to ensure that the file
handle's private_data points to a valid zfsdev_state_t.

Signed-off-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Andriy Gapon <avg@FreeBSD.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #3506
This commit is contained in:
Richard Yao 2015-04-16 09:20:02 -04:00 committed by Brian Behlendorf
parent 99b14de421
commit 72540ea314
4 changed files with 40 additions and 10 deletions

View File

@ -407,7 +407,7 @@ typedef struct zfsdev_state {
} zfsdev_state_t;
extern void *zfsdev_get_state(minor_t minor, enum zfsdev_state_type which);
extern minor_t zfsdev_getminor(struct file *filp);
extern int zfsdev_getminor(struct file *filp, minor_t *minorp);
extern minor_t zfsdev_minor_alloc(void);
#endif /* _KERNEL */

View File

@ -593,8 +593,9 @@ zfs_zevent_fd_hold(int fd, minor_t *minorp, zfs_zevent_t **ze)
if (fp == NULL)
return (EBADF);
*minorp = zfsdev_getminor(fp->f_file);
error = zfs_zevent_minor_to_state(*minorp, ze);
error = zfsdev_getminor(fp->f_file, minorp);
if (error == 0)
error = zfs_zevent_minor_to_state(*minorp, ze);
if (error)
zfs_zevent_fd_rele(fd);

View File

@ -5687,13 +5687,35 @@ zfsdev_get_state(minor_t minor, enum zfsdev_state_type which)
return (ptr);
}
minor_t
zfsdev_getminor(struct file *filp)
int
zfsdev_getminor(struct file *filp, minor_t *minorp)
{
ASSERT(filp != NULL);
ASSERT(filp->private_data != NULL);
zfsdev_state_t *zs, *fpd;
return (((zfsdev_state_t *)filp->private_data)->zs_minor);
ASSERT(filp != NULL);
ASSERT(!MUTEX_HELD(&zfsdev_state_lock));
fpd = filp->private_data;
if (fpd == NULL)
return (EBADF);
mutex_enter(&zfsdev_state_lock);
for (zs = zfsdev_state_list; zs != NULL; zs = zs->zs_next) {
if (zs->zs_minor == -1)
continue;
if (fpd == zs) {
*minorp = fpd->zs_minor;
mutex_exit(&zfsdev_state_lock);
return (0);
}
}
mutex_exit(&zfsdev_state_lock);
return (EBADF);
}
/*

View File

@ -126,13 +126,20 @@ zfs_onexit_fd_hold(int fd, minor_t *minorp)
{
file_t *fp;
zfs_onexit_t *zo;
int error;
fp = getf(fd);
if (fp == NULL)
return (SET_ERROR(EBADF));
*minorp = zfsdev_getminor(fp->f_file);
return (zfs_onexit_minor_to_state(*minorp, &zo));
error = zfsdev_getminor(fp->f_file, minorp);
if (error == 0)
error = zfs_onexit_minor_to_state(*minorp, &zo);
if (error)
zfs_onexit_fd_rele(fd);
return (error);
}
void