[PATCH 1/2] ceph: clamp the inline data length in ceph_fill_inline_data()

[PATCH 1/2] ceph: clamp the inline data length in ceph_fill_inline_data()

[Guanglei Zhu]

The MDS decides how much inline data to attach to a reply, and the
client parses inline_len without any upper bound: ceph_decode_need()
only verifies that the message actually carries that many bytes.
ceph_fill_inline_data() then memcpy()s the data into a single page
with no length check, so a malicious or buggy MDS returning more than
one page of inline data makes the client write past the end of the
page it allocated.

Clamp the length to PAGE_SIZE so both callers, handle_cap_grant() and
fill_inode(), are covered.

Fixes: 31c542a199d7 ("ceph: add inline data to pagecache")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---

Tested in a QEMU guest with a hacked MDS that reports an 8k inline
payload for a 4k file: without the clamp the client overwrites the
page behind the inline page and page poisoning complains on the next
allocation; with it the data is truncated and a warning is logged.
 fs/ceph/addr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index e598b2d42..795cd1b9e 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -2209,6 +2209,12 @@ void ceph_fill_inline_data(struct inode *inode, struct page *locked_page,
 	doutc(cl, "%p %llx.%llx len %zu locked_page %p\n", inode,
 	      ceph_vinop(inode), len, locked_page);
 
+	if (len > PAGE_SIZE) {
+		pr_warn_ratelimited_client(cl, "oversized inline data %zu\n",
+					   len);
+		len = PAGE_SIZE;
+	}
+
 	if (len > 0) {
 		void *kaddr = kmap_atomic(page);
 		memcpy(kaddr, data, len);
-- 
2.43.0

[PATCH 2/2] ceph: fix out-of-bounds read in ceph_netfs_issue_op_inline()

[Guanglei Zhu]

The read offset is validated against i_size but never against
inline_len, and the two fields come from the MDS independently.  When
a read starts past the end of the inline data, the subtraction

	len = min_t(size_t, iinfo->inline_len - subreq->start, subreq->len);

underflows and min_t() ends up with subreq->len, so copy_to_iter()
reads past the end of the inline buffer straight into the user
buffer.  A malicious or buggy MDS reporting a short inline payload
together with an inflated i_size can thus leak kernel heap memory to
userspace.

Bail out when the offset is not within the inline data.  The subreq
then completes short without progress and the read fails with
-ENODATA.

Fixes: 5b19f1eba459 ("ceph: make ceph_netfs_issue_op() handle inlined data")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---

Verified with a fault injector on the reply decoding path that
makes the MDS report a short inline payload with an inflated i_size:
the unpatched client trips HARDENED_USERCOPY on a read past the end
of the reply buffer, with the check the read fails with -ENODATA.
 fs/ceph/addr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 795cd1b9e..0db807abc 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -322,6 +322,11 @@ static bool ceph_netfs_issue_op_inline(struct netfs_io_subrequest *subreq)
 		return false;
 	}
 
+	if (subreq->start >= iinfo->inline_len) {
+		ceph_mdsc_put_request(req);
+		goto out;
+	}
+
 	len = min_t(size_t, iinfo->inline_len - subreq->start, subreq->len);
 	err = copy_to_iter(iinfo->inline_data + subreq->start, len, &subreq->io_iter);
 	if (err == 0) {
-- 
2.43.0

Re: [PATCH 1/2] ceph: clamp the inline data length in ceph_fill_inline_data()

[Alex Markuze]

Hi Guanglei,

NACK. Please respin.

  1. fs/ceph/addr.c: ceph_fill_inline_data (1/2)
     This does not apply to current testing. The helper already takes a
     folio and memcpy_to_folio(). Clamp to folio_size(folio), not a
     kmap_atomic() PAGE_SIZE page.

  2. fs/ceph/addr.c: ceph_netfs_issue_op_inline (2/2)
     The new start >= inline_len path leaves err at 0, so the subreq
     completes success with no progress. That is EOF for DIO, not the
     -ENODATA the commit message claims. Set an error before goto out.


-- 
Alex Markuze

[PATCH v2 1/2] ceph: clamp the inline data length in ceph_fill_inline_data()

[Guanglei Zhu]

The MDS decides how much inline data to attach to a reply, and the
client parses inline_len without any upper bound: ceph_decode_need()
only verifies that the message actually carries that many bytes.
ceph_fill_inline_data() then copies the data into a single folio
with no length check, so a malicious or buggy MDS returning more
inline data than the folio can hold writes past its end.

Clamp the length to folio_size() so both callers, handle_cap_grant()
and fill_inode(), are covered.

Fixes: 31c542a199d7 ("ceph: add inline data to pagecache")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---

- rebase onto ceph-client.git testing, where ceph_fill_inline_data()
  was converted to folios; clamp to folio_size() instead of PAGE_SIZE
  (reported by Alex Markuze)

 fs/ceph/addr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 4a2d3352a..3c7cf8a5e 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -2266,6 +2266,12 @@ void ceph_fill_inline_data(struct inode *inode, struct folio *locked_folio,
 	doutc(cl, "%p %llx.%llx len %zu locked_folio %p\n", inode,
 	      ceph_vinop(inode), len, locked_folio);
 
+	if (len > folio_size(folio)) {
+		pr_warn_ratelimited_client(cl, "oversized inline data %zu\n",
+					   len);
+		len = folio_size(folio);
+	}
+
 	if (len > 0)
 		memcpy_to_folio(folio, 0, data, len);
 
-- 
2.43.0

[PATCH v2 2/2] ceph: fix out-of-bounds read in ceph_netfs_issue_op_inline()

[Guanglei Zhu]

The read offset is validated against i_size but never against
inline_len, and the two fields come from the MDS independently.  When
a read starts past the end of the inline data, the subtraction

	len = min_t(size_t, iinfo->inline_len - subreq->start, subreq->len);

underflows and min_t() ends up with subreq->len, so copy_to_iter()
reads past the end of the inline buffer straight into the user
buffer.  A malicious or buggy MDS reporting a short inline payload
together with an inflated i_size can thus leak kernel heap memory to
userspace.

Bail out with -ENODATA when the offset is not within the inline
data.

Fixes: 5b19f1eba459 ("ceph: make ceph_netfs_issue_op() handle inlined data")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---

- set err = -ENODATA before bailing out on start >= inline_len, and
  correct the commit message: with err left at 0 the subreq completed
  with no progress, which netfs treats as EOF, not -ENODATA
  (reported by Alex Markuze)

 fs/ceph/addr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 3c7cf8a5e..dd33afca7 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -321,6 +321,12 @@ static bool ceph_netfs_issue_op_inline(struct netfs_io_subrequest *subreq)
 		return false;
 	}
 
+	if (subreq->start >= iinfo->inline_len) {
+		ceph_mdsc_put_request(req);
+		err = -ENODATA;
+		goto out;
+	}
+
 	len = min_t(size_t, iinfo->inline_len - subreq->start, subreq->len);
 	err = copy_to_iter(iinfo->inline_data + subreq->start, len, &subreq->io_iter);
 	if (err == 0) {
-- 
2.43.0