d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rework r252313:

Browse Source 
The filedesc lock may not be dropped unconditionally before exporting
fd to sbuf: fd might go away during execution.  While it is ok for
DTYPE_VNODE and DTYPE_FIFO because the export is from a vrefed vnode
here, for other types it is unsafe.

Instead, drop the lock in export_fd_to_sb(), after preparing data in
memory and before writing to sbuf.

Spotted by:	mjg
Suggested by:	kib
Review by:	kib
MFC after:	1 week
This commit is contained in:
Mikolaj Golub 2013-06-28 18:07:41 +00:00
parent c7328a9506
commit 6359d169ef
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=252349
1 changed files with 40 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
sys/kern/kern_descrip.c
View File

@ -3176,10 +3176,16 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC_OFILEDESC, ofiledesc, CTLFLAG_RD,
CTASSERT(sizeof(struct kinfo_file) == KINFO_FILE_SIZE);
#endif
struct export_fd_buf {
struct filedesc *fdp;
struct sbuf *sb;
ssize_t remainder;
struct kinfo_file kif;
};
static int
export_fd_to_sb(void *data, int type, int fd, int fflags, int refcnt,
int64_t offset, cap_rights_t fd_cap_rights, struct kinfo_file *kif,
struct sbuf *sb, ssize_t *remainder)
int64_t offset, cap_rights_t fd_cap_rights, struct export_fd_buf *efbuf)
{
struct {
int fflag;
@ -3202,16 +3208,23 @@ export_fd_to_sb(void *data, int type, int fd, int fflags, int refcnt,
{ O_TRUNC, KF_FLAG_TRUNC }
};
#define NFFLAGS (sizeof(fflags_table) / sizeof(*fflags_table))
struct kinfo_file *kif;
struct vnode *vp;
int error;
int error, locked;
unsigned int i;
if (*remainder == 0)
if (efbuf->remainder == 0)
return (0);
kif = &efbuf->kif;
bzero(kif, sizeof(*kif));
locked = efbuf->fdp != NULL;
switch (type) {
case KF_TYPE_FIFO:
case KF_TYPE_VNODE:
if (locked) {
FILEDESC_SUNLOCK(efbuf->fdp);
locked = 0;
}
vp = (struct vnode *)data;
error = fill_vnode_info(vp, kif);
vrele(vp);
@ -3255,15 +3268,19 @@ export_fd_to_sb(void *data, int type, int fd, int fflags, int refcnt,
kif->kf_structsize = offsetof(struct kinfo_file, kf_path) +
strlen(kif->kf_path) + 1;
kif->kf_structsize = roundup(kif->kf_structsize, sizeof(uint64_t));
if (*remainder != -1) {
if (*remainder < kif->kf_structsize) {
if (efbuf->remainder != -1) {
if (efbuf->remainder < kif->kf_structsize) {
/* Terminate export. */
*remainder = 0;
efbuf->remainder = 0;
return (0);
}
*remainder -= kif->kf_structsize;
efbuf->remainder -= kif->kf_structsize;
}
error = sbuf_bcat(sb, kif, kif->kf_structsize);
if (locked)
FILEDESC_SUNLOCK(efbuf->fdp);
error = sbuf_bcat(efbuf->sb, kif, kif->kf_structsize);
if (efbuf->fdp != NULL)
FILEDESC_SLOCK(efbuf->fdp);
return (error);
}
@ -3277,18 +3294,16 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
{
struct file *fp;
struct filedesc *fdp;
struct kinfo_file *kif;
struct export_fd_buf *efbuf;
struct vnode *cttyvp, *textvp, *tracevp;
int64_t offset;
void *data;
ssize_t remainder;
int error, i;
int type, refcnt, fflags;
cap_rights_t fd_cap_rights;
PROC_LOCK_ASSERT(p, MA_OWNED);
remainder = maxlen;
/* ktrace vnode */
tracevp = p->p_tracevp;
if (tracevp != NULL)
@ -3306,46 +3321,44 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
}
fdp = fdhold(p);
PROC_UNLOCK(p);
kif = malloc(sizeof(*kif), M_TEMP, M_WAITOK);
efbuf = malloc(sizeof(*efbuf), M_TEMP, M_WAITOK);
efbuf->fdp = NULL;
efbuf->sb = sb;
efbuf->remainder = maxlen;
if (tracevp != NULL)
export_fd_to_sb(tracevp, KF_TYPE_VNODE, KF_FD_TYPE_TRACE,
FREAD | FWRITE, -1, -1, 0, kif, sb, &remainder);
FREAD | FWRITE, -1, -1, 0, efbuf);
if (textvp != NULL)
export_fd_to_sb(textvp, KF_TYPE_VNODE, KF_FD_TYPE_TEXT,
FREAD, -1, -1, 0, kif, sb, &remainder);
FREAD, -1, -1, 0, efbuf);
if (cttyvp != NULL)
export_fd_to_sb(cttyvp, KF_TYPE_VNODE, KF_FD_TYPE_CTTY,
FREAD | FWRITE, -1, -1, 0, kif, sb, &remainder);
FREAD | FWRITE, -1, -1, 0, efbuf);
error = 0;
if (fdp == NULL)
goto fail;
efbuf->fdp = fdp;
FILEDESC_SLOCK(fdp);
/* working directory */
if (fdp->fd_cdir != NULL) {
vref(fdp->fd_cdir);
data = fdp->fd_cdir;
FILEDESC_SUNLOCK(fdp);
export_fd_to_sb(data, KF_TYPE_VNODE, KF_FD_TYPE_CWD,
FREAD, -1, -1, 0, kif, sb, &remainder);
FILEDESC_SLOCK(fdp);
FREAD, -1, -1, 0, efbuf);
}
/* root directory */
if (fdp->fd_rdir != NULL) {
vref(fdp->fd_rdir);
data = fdp->fd_rdir;
FILEDESC_SUNLOCK(fdp);
export_fd_to_sb(data, KF_TYPE_VNODE, KF_FD_TYPE_ROOT,
FREAD, -1, -1, 0, kif, sb, &remainder);
FILEDESC_SLOCK(fdp);
FREAD, -1, -1, 0, efbuf);
}
/* jail directory */
if (fdp->fd_jdir != NULL) {
vref(fdp->fd_jdir);
data = fdp->fd_jdir;
FILEDESC_SUNLOCK(fdp);
export_fd_to_sb(data, KF_TYPE_VNODE, KF_FD_TYPE_JAIL,
FREAD, -1, -1, 0, kif, sb, &remainder);
FILEDESC_SLOCK(fdp);
FREAD, -1, -1, 0, efbuf);
}
for (i = 0; i < fdp->fd_nfiles; i++) {
if ((fp = fdp->fd_ofiles[i].fde_file) == NULL)
@ -3427,10 +3440,8 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
* re-validate and re-evaluate its properties when
* the loop continues.
*/
FILEDESC_SUNLOCK(fdp);
error = export_fd_to_sb(data, type, i, fflags, refcnt,
offset, fd_cap_rights, kif, sb, &remainder);
FILEDESC_SLOCK(fdp);
offset, fd_cap_rights, efbuf);
if (error)
break;
}
@ -3438,7 +3449,7 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen)
fail:
if (fdp != NULL)
fddrop(fdp);
free(kif, M_TEMP);
free(efbuf, M_TEMP);
return (error);
}