From cd65363eb9593c640d85e4915d214051aa5bc5f2 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Mon, 15 Jun 2009 14:50:06 -0700 Subject: [PATCH] Set tty ownership on pty open (temporary until devfs is integrated) Fix a very long standing issue when opening new pty's as non-root. The related tty could not be chowned/chmoded in that case, giving any user in the system access to the pty. This would not effect programs like sshd which set up the pty as root, but it did effect programs like xterm which do not run suid-root. This is strictly a temporary fix until devfs can be integrated into the system. Basically we allow the uid stored in the cdev_t structure to override the uid returned by VOP_GETATTR for VCHR devices, and the chmod helper function checks the uid stored in the cdev_t structure for compatibility. Reported-by: Hasso Tepper , Numerous others --- sys/kern/kern_conf.c | 1 + sys/kern/tty_pty.c | 2 ++ sys/kern/vfs_helper.c | 23 +++++++++++++++++++++++ sys/sys/conf.h | 5 +++++ sys/vfs/hammer/hammer_vnops.c | 20 +++++++++++++++++++- sys/vfs/specfs/spec_vnops.c | 33 +++++++++++++++++++++++++++++++++ sys/vfs/ufs/ufs_vnops.c | 29 ++++++++++++++++++++++++++--- 7 files changed, 109 insertions(+), 4 deletions(-) diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c index c99b38a8eb..cc86d66b2a 100644 --- a/sys/kern/kern_conf.c +++ b/sys/kern/kern_conf.c @@ -292,6 +292,7 @@ make_dev(struct dev_ops *ops, int minor, uid_t uid, gid_t gid, __va_start(ap, fmt); i = kvcprintf(fmt, NULL, dev->si_name, 32, ap); dev->si_name[i] = '\0'; + dev->si_uid = uid; __va_end(ap); return (dev); diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c index dafe757444..84c61cb238 100644 --- a/sys/kern/tty_pty.c +++ b/sys/kern/tty_pty.c @@ -349,6 +349,7 @@ ptcopen(struct dev_open_args *ap) pti->pt_flags = 0; pti->pt_send = 0; pti->pt_ucntl = 0; + pti->devs->si_uid = ap->a_cred->cr_uid; return (0); } @@ -373,6 +374,7 @@ ptcclose(struct dev_close_args *ap) tp->t_state &= ~(TS_CARR_ON | TS_CONNECTED); tp->t_state |= TS_ZOMBIE; ttyflush(tp, FREAD | FWRITE); + tp->t_dev->si_uid = 0; } tp->t_oproc = 0; /* mark closed */ diff --git a/sys/kern/vfs_helper.c b/sys/kern/vfs_helper.c index 1b2f6a53a2..f1c12dbbcf 100644 --- a/sys/kern/vfs_helper.c +++ b/sys/kern/vfs_helper.c @@ -46,6 +46,7 @@ #include #include +#include #include #include #include @@ -190,12 +191,34 @@ vop_helper_create_uid(struct mount *mp, mode_t dmode, uid_t duid, /* * This helper may be used by VFSs to implement unix chmod semantics. + * + * XXX TEMPORARY chmod override for VCHR devices. Allow the device to + * override the uid if the uid is not root. This is used + * by pty's to set the owner for the related tty. + * + * XXX REMOVE THE OVERRIDE WHEN DEVFS IS MERGED. */ int vop_helper_chmod(struct vnode *vp, mode_t new_mode, struct ucred *cred, uid_t cur_uid, gid_t cur_gid, mode_t *cur_modep) { int error; + cdev_t dev; + + /* + * HACK to support openpty(). If called by root openpty() chown's + * the tty and we allow this. If not called by root the kernel + * saves the uid in si_uid and if the tty is not owned by root we + * override it here. + * + * NOTE: The vnode might not be open so v_rdev might not be assigned. + */ + if (vp->v_type == VCHR && cur_uid == 0) { + if ((dev = vp->v_rdev) == NULL) + dev = get_dev(vp->v_umajor, vp->v_uminor); + if (dev) + cur_uid = dev->si_uid; + } if (cred->cr_uid != cur_uid) { error = priv_check_cred(cred, PRIV_ROOT, PRISON_ROOT); diff --git a/sys/sys/conf.h b/sys/sys/conf.h index 666994fea2..a992615721 100644 --- a/sys/sys/conf.h +++ b/sys/sys/conf.h @@ -67,6 +67,11 @@ struct dev_ops; struct cdev { u_int si_flags; + __uint64_t si_inode; + uid_t si_uid; + gid_t si_gid; + int si_perms; + TAILQ_ENTRY(cdev) link; int si_uminor; int si_umajor; LIST_ENTRY(cdev) si_hash; diff --git a/sys/vfs/hammer/hammer_vnops.c b/sys/vfs/hammer/hammer_vnops.c index bcb481f06d..adc9782e84 100644 --- a/sys/vfs/hammer/hammer_vnops.c +++ b/sys/vfs/hammer/hammer_vnops.c @@ -91,6 +91,7 @@ static int hammer_vop_fifokqfilter (struct vop_kqfilter_args *); static int hammer_vop_specclose (struct vop_close_args *); static int hammer_vop_specread (struct vop_read_args *); static int hammer_vop_specwrite (struct vop_write_args *); +static int hammer_vop_specgetattr (struct vop_getattr_args *); struct vop_ops hammer_vnode_vops = { .vop_default = vop_defaultop, @@ -138,7 +139,7 @@ struct vop_ops hammer_spec_vops = { .vop_access = hammer_vop_access, .vop_close = hammer_vop_specclose, .vop_markatime = hammer_vop_markatime, - .vop_getattr = hammer_vop_getattr, + .vop_getattr = hammer_vop_specgetattr, .vop_inactive = hammer_vop_inactive, .vop_reclaim = hammer_vop_reclaim, .vop_setattr = hammer_vop_setattr @@ -2979,6 +2980,23 @@ hammer_vop_specwrite (struct vop_write_args *ap) return (VOCALL(&spec_vnode_vops, &ap->a_head)); } +/* + * SPECFS's getattr will override fields as necessary, but does not fill + * stuff in from scratch. + */ +static +int +hammer_vop_specgetattr (struct vop_getattr_args *ap) +{ + int error; + + error = hammer_vop_getattr(ap); + if (error == 0) + VOCALL(&spec_vnode_vops, &ap->a_head); + return (error); +} + + /************************************************************************ * KQFILTER OPS * ************************************************************************ diff --git a/sys/vfs/specfs/spec_vnops.c b/sys/vfs/specfs/spec_vnops.c index 91fbbfdd28..c15e0971d7 100644 --- a/sys/vfs/specfs/spec_vnops.c +++ b/sys/vfs/specfs/spec_vnops.c @@ -86,6 +86,7 @@ static int spec_read (struct vop_read_args *); static int spec_strategy (struct vop_strategy_args *); static int spec_write (struct vop_write_args *); static void spec_strategy_done(struct bio *nbio); +static int spec_getattr (struct vop_getattr_args *); struct vop_ops spec_vnode_vops = { .vop_default = vop_defaultop, @@ -116,6 +117,7 @@ struct vop_ops spec_vnode_vops = { .vop_old_rename = (void *)vop_panic, .vop_old_rmdir = (void *)vop_panic, .vop_setattr = (void *)vop_ebadf, + .vop_getattr = spec_getattr, .vop_strategy = spec_strategy, .vop_old_symlink = (void *)vop_panic, .vop_write = spec_write @@ -371,6 +373,37 @@ spec_write(struct vop_write_args *ap) return (error); } +/* + * This call modifying an upper layer filesystem's idea of the owner only. + * It does NOT replace the upper layer filesystem's getattr. + * + * XXX spec_getattr() - TEMPORARY - remove when devfs is fully installed. + * This is a horrible hack. The code will get confused + * if root chown's a tty device and then fails to chown + * it back after finishing with it. Non-root chowns are + * strictly stored in the device and don't have the issue. + */ +static int +spec_getattr (struct vop_getattr_args *ap) +{ + struct vattr *vap = ap->a_vap; + struct vnode *vp = ap->a_vp; + cdev_t dev; + + if (vp->v_type != VCHR) + return (0); + if ((dev = vp->v_rdev) == NULL) { + if ((dev = vp->v_rdev) == NULL) + dev = get_dev(vp->v_umajor, vp->v_uminor); + if (dev == NULL) + return (0); + } + if (vap->va_uid == 0) { + vap->va_uid = dev->si_uid; + } + return (0); +} + /* * Device ioctl operation. * diff --git a/sys/vfs/ufs/ufs_vnops.c b/sys/vfs/ufs/ufs_vnops.c index 14ca305acb..09423fdfc1 100644 --- a/sys/vfs/ufs/ufs_vnops.c +++ b/sys/vfs/ufs/ufs_vnops.c @@ -111,6 +111,7 @@ static int ufsfifo_write (struct vop_write_args *); static int ufsspec_close (struct vop_close_args *); static int ufsspec_read (struct vop_read_args *); static int ufsspec_write (struct vop_write_args *); +static int ufsspec_getattr (struct vop_getattr_args *); static int filt_ufsread (struct knote *kn, long hint); static int filt_ufswrite (struct knote *kn, long hint); static int filt_ufsvnode (struct knote *kn, long hint); @@ -567,7 +568,13 @@ ufs_chmod(struct vnode *vp, int mode, struct ucred *cred) { struct inode *ip = VTOI(vp); int error; + mode_t cur_mode = ip->i_mode; + error = vop_helper_chmod(vp, mode, cred, ip->i_uid, ip->i_gid, + &cur_mode); + if (error) + return (error); +#if 0 if (cred->cr_uid != ip->i_uid) { error = priv_check_cred(cred, PRIV_ROOT, PRISON_ROOT); if (error) @@ -579,8 +586,8 @@ ufs_chmod(struct vnode *vp, int mode, struct ucred *cred) if (!groupmember(ip->i_gid, cred) && (mode & ISGID)) return (EPERM); } - ip->i_mode &= ~ALLPERMS; - ip->i_mode |= (mode & ALLPERMS); +#endif + ip->i_mode = cur_mode; ip->i_flag |= IN_CHANGE; return (0); } @@ -1937,6 +1944,22 @@ ufsspec_write(struct vop_write_args *ap) return (error); } +/* + * SPECFS's getattr will override fields as necessary, but does not fill + * stuff in from scratch. + */ +static +int +ufsspec_getattr (struct vop_getattr_args *ap) +{ + int error; + + error = ufs_getattr(ap); + if (error == 0) + VOCALL(&spec_vnode_vops, &ap->a_head); + return (error); +} + /* * Close wrapper for special devices. * @@ -2383,7 +2406,7 @@ static struct vop_ops ufs_spec_vops = { .vop_fsync = (void *)ufs_missingop, .vop_access = ufs_access, .vop_close = ufsspec_close, - .vop_getattr = ufs_getattr, + .vop_getattr = ufsspec_getattr, .vop_inactive = ufs_inactive, .vop_print = ufs_print, .vop_read = ufsspec_read, -- 2.41.0