From nobody Tue Apr 19 01:01:35 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 25FDB11E169F; Tue, 19 Apr 2022 01:01:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Kj59D0c8Rz4YQM; Tue, 19 Apr 2022 01:01:36 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1650330096; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=P/iz+riq1DIxn6TzVi8CAfjKaMvMLItqsdQUhtApokE=; b=v4VBAZxXeigBEDsXQ4gnslAaXM+u0h2K3J0U1Yg/u58xKREhDPG/ts6m02m/D86YLt4L7+ QqtzUFpjfJHtlAx/bQxxieH+IqWEbZxsgK5WseMVwD2DZN14PNppwpbUiIyeWTjSZc3ucP r5dhV1mq4bWonoUB6mhw+O3O+V8OG5MErF32j0cJp9kQyuNjpJS7NBElXngL8zyfvEtr2W fbDweFoatjUM7bCuV+WnlkmEj0MS9Z0ZnFy9/sUM+9nE/xbngPJEqcdo+sG6dT9v5Az+p/ DfyDCMEJ4ugHbttvBS9FZ6Vk76ORBy7oU4hlg7+oPNNQZMNmNmRejylCT5U5Rg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E9F4B1DB65; Tue, 19 Apr 2022 01:01:35 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 23J11ZTa073796; Tue, 19 Apr 2022 01:01:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 23J11ZG9073795; Tue, 19 Apr 2022 01:01:35 GMT (envelope-from git) Date: Tue, 19 Apr 2022 01:01:35 GMT Message-Id: <202204190101.23J11ZG9073795@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Alan Somers Subject: git: 3a1b3c6a1e68 - main - fusefs: correctly handle servers that report too much data written List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: asomers X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 3a1b3c6a1e68063330e897a5a5c94518edae4a3b Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1650330096; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=P/iz+riq1DIxn6TzVi8CAfjKaMvMLItqsdQUhtApokE=; b=oOCH71Ii7u7Cc8gAzOSYf+R/5Fvq81SNMSLh0W08XOJkECEm0G+wCywDdF+sgxiVnznkfV IU5i0b95cjzYyUkA9RL9XD5wBCihVm26UgN2nbPxc62ylHM5tu+sB4qCHxLjxeL4+KW1pP tfL7WR2raoQO+6E2C+rO1Rvgt497XNAiKOKKHUofryXuGtv50yahzrCiDrSccRy6SmXn21 PUXThwi3T6Zt/6DqExklPkLJ/B4p9fLX4Fp/kThwwkR95R3bDBYCmtWUeAjJdKxprCF5n4 a+ccJ7byfWPnCiUyc/N53ON+IPlG16vdGY2d470r/JocJWRHPzyUDimWHw7/RQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1650330096; a=rsa-sha256; cv=none; b=Km7Fq8Qu5Z6sMsXF6k2FYAKWsJ0129tB9IniKfai1yA1oB793i752sZ7YkTrK/swKTKi00 GwMsTcAUPgxi5hsNwH9WKODu5VKUHGDmnvEvxlQ8MQ24frjUXVdb9lbo41xZDUrif8mkit RMlC8dXvvsIhJfnpDSJGpVuGV1VhnuR5jEdkOrfC52ZccxwZiiCALub3+ArKSHKIRqZlpO p3YZ7FQU6Sh+sehqWHA4DfcRBV2DL0QoguaqmhhYhLa87ufJZ4wQkFtgEvDgCoZY/dRifs HuiTZsL/0CpXyUjfK30zDUnFj0ns2LX2nDsygc9Vwuasyj2Iod6AtlCuJ9Gftg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=3a1b3c6a1e68063330e897a5a5c94518edae4a3b commit 3a1b3c6a1e68063330e897a5a5c94518edae4a3b Author: Alan Somers AuthorDate: 2022-04-18 23:03:53 +0000 Commit: Alan Somers CommitDate: 2022-04-19 00:59:10 +0000 fusefs: correctly handle servers that report too much data written During a FUSE_WRITE, the kernel requests the server to write a certain amount of data, and the server responds with the amount that it actually did write. It is obviously an error for the server to write more than it was provided, and we always treated it as such, but there were two problems: * If the server responded with a huge amount, greater than INT_MAX, it would trigger an integer overflow which would cause a panic. * When extending the file, we wrongly set the file's size before validing the amount written. PR: 263263 Reported by: Robert Morris MFC after: 2 weeks Sponsored by: Axcient Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D34955 --- sys/fs/fuse/fuse_io.c | 18 ++++++++----- tests/sys/fs/fusefs/write.cc | 61 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c index 07c0f958e028..9ef4badfcdff 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -395,8 +395,19 @@ retry: fwo = ((struct fuse_write_out *)fdi.answ); + if (fwo->size > fwi->size) { + fuse_warn(data, FSESS_WARN_WROTE_LONG, + "wrote more data than we provided it."); + /* This is bonkers. Clear attr cache. */ + fvdat->flag &= ~FN_SIZECHANGE; + fuse_vnode_clear_attr_cache(vp); + err = EINVAL; + break; + } + /* Adjust the uio in the case of short writes */ diff = fwi->size - fwo->size; + as_written_offset = uio->uio_offset - diff; if (as_written_offset - diff > filesize) { @@ -406,12 +417,7 @@ retry: if (as_written_offset - diff >= filesize) fvdat->flag &= ~FN_SIZECHANGE; - if (diff < 0) { - fuse_warn(data, FSESS_WARN_WROTE_LONG, - "wrote more data than we provided it."); - err = EINVAL; - break; - } else if (diff > 0) { + if (diff > 0) { /* Short write */ if (!direct_io) { fuse_warn(data, FSESS_WARN_SHORT_WRITE, diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc index db5bb3fe4441..d685bd13aa17 100644 --- a/tests/sys/fs/fusefs/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -410,6 +410,67 @@ TEST_F(Write, indirect_io_short_write) leak(fd); } +/* It is an error if the daemon claims to have written more data than we sent */ +TEST_F(Write, indirect_io_long_write) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefghijklmnop"; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + ssize_t bufsize_out = 100; + off_t some_other_size = 25; + struct stat sb; + + expect_lookup(RELPATH, ino, 0); + expect_open(ino, 0, 1); + expect_write(ino, 0, bufsize, bufsize_out, CONTENTS); + expect_getattr(ino, some_other_size); + + fd = open(FULLPATH, O_WRONLY); + ASSERT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(-1, write(fd, CONTENTS, bufsize)) << strerror(errno); + ASSERT_EQ(EINVAL, errno); + + /* + * Following such an error, we should requery the server for the file's + * size. + */ + fstat(fd, &sb); + ASSERT_EQ(sb.st_size, some_other_size); + + leak(fd); +} + +/* + * Don't crash if the server returns a write that can't be represented as a + * signed 32 bit number. Regression test for + * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263263 + */ +TEST_F(Write, indirect_io_very_long_write) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefghijklmnop"; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + ssize_t bufsize_out = 3 << 30; + + expect_lookup(RELPATH, ino, 0); + expect_open(ino, 0, 1); + expect_write(ino, 0, bufsize, bufsize_out, CONTENTS); + + fd = open(FULLPATH, O_WRONLY); + ASSERT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(-1, write(fd, CONTENTS, bufsize)) << strerror(errno); + ASSERT_EQ(EINVAL, errno); + leak(fd); +} + /* * When the direct_io option is used, filesystems are allowed to write less * data than requested. We should return the short write to userland.