linux-kernel - Re: [PATCH v2] fs/ceph/file: fix buffer overflow in __ceph_sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAO8a2SiUL3T=MHcktWDaMbToqJYt7mYD_XN5G2nRAN0sxCHD7w@mail.gmail.com>
Date: Wed, 27 Nov 2024 22:56:53 +0200
From: Alex Markuze <amarkuze@...hat.com>
To: Max Kellermann <max.kellermann@...os.com>
Cc: xiubli@...hat.com, idryomov@...il.com, ceph-devel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] fs/ceph/file: fix buffer overflow in __ceph_sync_read()

You are correct, that is why I'm testing a patch that deals with all
cases where i_size < offset.
I will CC you on the other thread.

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 4b8d59ebda00..19b084212fee 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1066,7 +1066,7 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
        if (ceph_inode_is_shutdown(inode))
                return -EIO;

-       if (!len)
+       if (!len || !i_size)
                return 0;
        /*
         * flush any page cache pages in this range.  this
@@ -1200,12 +1200,11 @@ ssize_t __ceph_sync_read(struct inode *inode,
loff_t *ki_pos,
                }

                idx = 0;
-               if (ret <= 0)
-                       left = 0;
-               else if (off + ret > i_size)
-                       left = i_size - off;
+               if (off + ret > i_size)
+                       left = (i_size > off) ? i_size - off : 0;
                else
-                       left = ret;
+                       left = (ret > 0) ? ret : 0;
+
                while (left > 0) {
                        size_t plen, copied;



On Wed, Nov 27, 2024 at 10:43 PM Max Kellermann
<max.kellermann@...os.com> wrote:
>
> On Wed, Nov 27, 2024 at 9:40 PM Alex Markuze <amarkuze@...hat.com> wrote:
> > There is a fix for this proposed by Luis.
>
> On the private security mailing list, I wrote about it:
> "This patch is incomplete because it only checks for i_size==0.
> Truncation to zero is the most common case, but any situation where
> offset is suddenly larger than the new size triggers this bug."
>
> I think my patch is better.
>