Author: trociny
Date: Fri Jun 28 18:07:41 2013
New Revision: 252349
URL: http://svnweb.freebsd.org/changeset/base/252349
Log:
Rework r252313:
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
Modified:
head/sys/kern/kern_descrip.c
Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c Fri Jun 28 17:18:28 2013
(r252348)
+++ head/sys/kern/kern_descrip.c Fri Jun 28 18:07:41 2013
(r252349)
@@ -3176,10 +3176,16 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC
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, in
{ 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, in
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 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,
}
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,
* 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,
fail:
if (fdp != NULL)
fddrop(fdp);
- free(kif, M_TEMP);
+ free(efbuf, M_TEMP);
return (error);
}
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
