Differential D28605 Diff 83811 sys/fs/fuse/fuse_internal.c

Changeset View

Standalone View

sys/fs/fuse/fuse_internal.c

Show First 20 Lines • Show All 577 Lines • ▼ Show 20 Lines	fuse_internal_readdir_processdata(struct uio *uio,
size_t reqsize,		size_t reqsize,
void *buf,		void *buf,
size_t bufsize,		size_t bufsize,
struct fuse_iov *cookediov,		struct fuse_iov *cookediov,
int *ncookies,		int *ncookies,
u_long **cookiesp)		u_long **cookiesp)
{		{
int err = 0;		int err = 0;
int bytesavail;		int oreclen;
size_t freclen;		size_t freclen;

struct dirent *de;		struct dirent *de;
struct fuse_dirent *fudge;		struct fuse_dirent *fudge;
u_long *cookies;		u_long *cookies;

cookies = *cookiesp;		cookies = *cookiesp;
if (bufsize < FUSE_NAME_OFFSET)		if (bufsize < FUSE_NAME_OFFSET)
Show All 20 Lines	if (isbzero(buf, FUSE_NAME_OFFSET)) {
break;		break;
}		}
#endif		#endif

if (!fudge->namelen \|\| fudge->namelen > MAXNAMLEN) {		if (!fudge->namelen \|\| fudge->namelen > MAXNAMLEN) {
err = EINVAL;		err = EINVAL;
break;		break;
}		}
bytesavail = GENERIC_DIRSIZ((struct pseudo_dirent *)		oreclen = GENERIC_DIRSIZ((struct pseudo_dirent *)
&fudge->namelen);		&fudge->namelen);
		cemUnsubmitted Done Inline Actions `bytesavail` is a bad name for this. Maybe `oreclen` for output reclen? cem: `bytesavail` is a bad name for this. Maybe `oreclen` for output reclen?
		asomersAuthorUnsubmitted Done Inline Actions Yeah, I agree. asomers: Yeah, I agree.

if (bytesavail > uio_resid(uio)) {		if (oreclen > uio_resid(uio)) {
/* Out of space for the dir so we are done. */		/* Out of space for the dir so we are done. */
err = -1;		err = -1;
break;		break;
}		}
/*		/*
* Don't start to copy the directory entries out until		* Don't start to copy the directory entries out until
* the requested offset in the directory is found.		* the requested offset in the directory is found.
*/		*/
if (*fnd_start != 0) {		if (*fnd_start != 0) {
fiov_adjust(cookediov, bytesavail);		fiov_adjust(cookediov, oreclen);
bzero(cookediov->base, bytesavail);		bzero(cookediov->base, oreclen);

de = (struct dirent *)cookediov->base;		de = (struct dirent *)cookediov->base;
de->d_fileno = fudge->ino;		de->d_fileno = fudge->ino;
de->d_reclen = bytesavail;		de->d_off = fudge->off;
		cemUnsubmitted Done Inline Actions So, for FreeBSD, I think `d_off` is expected to be the offset of the next `dirent` in the logical stream. I.e., `d_off = nbytes_already_emitted + bytesavail`. It's a little weird for us because we're translating between a FUSE-formatted stream and a `dirent`-formatted output. I think normally I'd say offsets and seeks on the stream from FreeBSD should be in terms of the `dirent` output rather than the `fuse_dirent` input, but that makes `lseek` from userspace to FUSE basically impossible (with some manual cache internal to FUSE -- ugly). I think we should (1) keep a running total of `nbytes_already_read` = sum(`freclen`), and (2) validate that `fudge->off == nbytes_already_read + freclen` (or whatever is appropriate for FUSE semantics -- maybe it should just be `nbytes_already_read`) to verify the internal consistency of the FUSE dirent stream we're getting from the untrusted server. cem: So, for FreeBSD, I think `d_off` is expected to be the offset of the next `dirent` in the…
		asomersAuthorUnsubmitted Done Inline Actions You're making assumptions about what `d_off` means. What you describe makes sense if the stream is a compact sequence of dirents, as it might be for UFS. But the FUSE server is allowed to interpret `d_off` however it wishes. I don't think we have any choice but to accept whatever the server tells us. The worst that happens is that the user ends up seeking to an invalid offset. But only the server knows which offsets are invalid. asomers: You're making assumptions about what `d_off` means. What you describe makes sense if the…
		cemUnsubmitted Not Done Inline Actions Ok cem: Ok
		de->d_reclen = oreclen;
de->d_type = fudge->type;		de->d_type = fudge->type;
de->d_namlen = fudge->namelen;		de->d_namlen = fudge->namelen;
memcpy((char *)cookediov->base + sizeof(struct dirent) -		memcpy((char *)cookediov->base + sizeof(struct dirent) -
MAXNAMLEN - 1,		MAXNAMLEN - 1,
(char *)buf + FUSE_NAME_OFFSET, fudge->namelen);		(char *)buf + FUSE_NAME_OFFSET, fudge->namelen);
dirent_terminate(de);		dirent_terminate(de);

err = uiomove(cookediov->base, cookediov->len, uio);		err = uiomove(cookediov->base, cookediov->len, uio);
if (err)		if (err)
break;		break;
if (cookies != NULL) {		if (cookies != NULL) {
if (*ncookies == 0) {		if (*ncookies == 0) {
err = -1;		err = -1;
break;		break;
}		}
*cookies = fudge->off;		*cookies = fudge->off;
cookies++;		cookies++;
(*ncookies)--;		(*ncookies)--;
}		}
} else if (startoff == fudge->off)		} else if (startoff == fudge->off)
*fnd_start = 1;		*fnd_start = 1;
		cemUnsubmitted Done Inline Actions This check should go before the `if (fnd_start != 0)` fill clause -- `startoff` is inclusive. cem:* This check should go before the `if (*fnd_start != 0)` fill clause -- `startoff` is inclusive.
		asomersAuthorUnsubmitted Done Inline Actions No, the existing placement is correct. `fudge->off` is the seek offset of the _next_ dirent, not the current one. asomers: No, the existing placement is correct. `fudge->off` is the seek offset of the _next_ dirent…
		cemUnsubmitted Not Done Inline Actions I'm not sure what I was thinking, you're right. cem: I'm not sure what I was thinking, you're right.
buf = (char *)buf + freclen;		buf = (char *)buf + freclen;
bufsize -= freclen;		bufsize -= freclen;
uio_setoffset(uio, fudge->off);		uio_setoffset(uio, fudge->off);
}		}
*cookiesp = cookies;		*cookiesp = cookies;

return err;		return err;
}		}
▲ Show 20 Lines • Show All 637 Lines • Show Last 20 Lines