fusefs: validate servers' error values
Formerly fusefs would pass up the stack any error value returned by the fuse server. However, some values aren't valid for userland, but have special meanings within the kernel. One of these, EJUSTRETURN, could cause a kernel page fault if the server returned it in response to FUSE_LOOKUP. Fix by validating all errors returned by the server. Also, fix a data lifetime bug in the FUSE_DESTROY test. PR: 263220 Reported by: Robert Morris <rtm@lcs.mit.edu> MFC after: 3 weeks Sponsored by: Axcient Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D34931
This commit is contained in:
parent
a812a5cacb
commit
155ac516c6
@ -518,8 +518,18 @@ fuse_device_write(struct cdev *dev, struct uio *uio, int ioflag)
|
||||
"pass ticket to a callback");
|
||||
/* Sanitize the linuxism of negative errnos */
|
||||
ohead.error *= -1;
|
||||
memcpy(&tick->tk_aw_ohead, &ohead, sizeof(ohead));
|
||||
err = tick->tk_aw_handler(tick, uio);
|
||||
if (ohead.error < 0 || ohead.error > ELAST) {
|
||||
/* Illegal error code */
|
||||
ohead.error = EIO;
|
||||
memcpy(&tick->tk_aw_ohead, &ohead,
|
||||
sizeof(ohead));
|
||||
tick->tk_aw_handler(tick, uio);
|
||||
err = EINVAL;
|
||||
} else {
|
||||
memcpy(&tick->tk_aw_ohead, &ohead,
|
||||
sizeof(ohead));
|
||||
err = tick->tk_aw_handler(tick, uio);
|
||||
}
|
||||
} else {
|
||||
/* pretender doesn't wanna do anything with answer */
|
||||
SDT_PROBE2(fusefs, , device, trace, 1,
|
||||
|
@ -276,6 +276,27 @@ TEST_F(Lookup, dotdot_no_parent_nid)
|
||||
EXPECT_EQ(ESTALE, errno);
|
||||
}
|
||||
|
||||
/*
|
||||
* A daemon that returns an illegal error value should be handled gracefully.
|
||||
* Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263220
|
||||
*/
|
||||
TEST_F(Lookup, ejustreturn)
|
||||
{
|
||||
const char FULLPATH[] = "mountpoint/does_not_exist";
|
||||
const char RELPATH[] = "does_not_exist";
|
||||
|
||||
EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
|
||||
.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
|
||||
out.header.len = sizeof(out.header);
|
||||
out.header.error = 2;
|
||||
m_mock->m_expected_write_errno = EINVAL;
|
||||
})));
|
||||
|
||||
EXPECT_NE(0, access(FULLPATH, F_OK));
|
||||
|
||||
EXPECT_EQ(EIO, errno);
|
||||
}
|
||||
|
||||
TEST_F(Lookup, enoent)
|
||||
{
|
||||
const char FULLPATH[] = "mountpoint/does_not_exist";
|
||||
|
@ -418,6 +418,7 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
|
||||
const bool trueval = true;
|
||||
|
||||
m_daemon_id = NULL;
|
||||
m_expected_write_errno = 0;
|
||||
m_kernel_minor_version = kernel_minor_version;
|
||||
m_maxreadahead = max_readahead;
|
||||
m_maxwrite = MIN(max_write, max_max_write);
|
||||
@ -779,6 +780,7 @@ void MockFS::loop() {
|
||||
|
||||
bzero(in.get(), sizeof(*in));
|
||||
read_request(*in, buflen);
|
||||
m_expected_write_errno = 0;
|
||||
if (m_quit)
|
||||
break;
|
||||
if (verbosity > 0)
|
||||
@ -1011,7 +1013,12 @@ void MockFS::write_response(const mockfs_buf_out &out) {
|
||||
FAIL() << "not yet implemented";
|
||||
}
|
||||
r = write(m_fuse_fd, &out, out.header.len);
|
||||
ASSERT_TRUE(r > 0 || errno == EAGAIN) << strerror(errno);
|
||||
if (m_expected_write_errno) {
|
||||
ASSERT_EQ(-1, r);
|
||||
ASSERT_EQ(m_expected_write_errno, errno) << strerror(errno);
|
||||
} else {
|
||||
ASSERT_TRUE(r > 0 || errno == EAGAIN) << strerror(errno);
|
||||
}
|
||||
}
|
||||
|
||||
void* MockFS::service(void *pthr_data) {
|
||||
|
@ -340,6 +340,9 @@ class MockFS {
|
||||
/* pid of child process, for two-process test cases */
|
||||
pid_t m_child_pid;
|
||||
|
||||
/* the expected errno of the next write to /dev/fuse */
|
||||
int m_expected_write_errno;
|
||||
|
||||
/* Maximum size of a FUSE_WRITE write */
|
||||
uint32_t m_maxwrite;
|
||||
|
||||
|
@ -217,7 +217,7 @@ FuseTest::expect_destroy(int error)
|
||||
return (in.header.opcode == FUSE_DESTROY);
|
||||
}, Eq(true)),
|
||||
_)
|
||||
).WillOnce(Invoke( ReturnImmediate([&](auto in, auto& out) {
|
||||
).WillOnce(Invoke(ReturnImmediate([=](auto in, auto& out) {
|
||||
m_mock->m_quit = true;
|
||||
out.header.len = sizeof(out.header);
|
||||
out.header.unique = in.header.unique;
|
||||
|
Loading…
Reference in New Issue
Block a user