linux-kernel - [RFC][arm64] possible infinite loop in btrfs search

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]

Date:   Sat, 28 Aug 2021 19:28:17 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andreas Gruenbacher <agruenba@...hat.com>,
        Christoph Hellwig <hch@...radead.org>,
        "Darrick J. Wong" <djwong@...nel.org>, Jan Kara <jack@...e.cz>,
        Matthew Wilcox <willy@...radead.org>,
        cluster-devel <cluster-devel@...hat.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        ocfs2-devel@....oracle.com, Josef Bacik <josef@...icpanda.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>
Subject: [RFC][arm64] possible infinite loop in btrfs search_ioctl()

	AFAICS, a48b73eca4ce "btrfs: fix potential deadlock in the search ioctl"
has introduced a bug at least on arm64.

Relevant bits: in search_ioctl() we have
        while (1) {
                ret = fault_in_pages_writeable(ubuf + sk_offset,
                                               *buf_size - sk_offset);
                if (ret)
                        break;

                ret = btrfs_search_forward(root, &key, path, sk->min_transid);
                if (ret != 0) {
                        if (ret > 0)
                                ret = 0;
                        goto err;
                }
                ret = copy_to_sk(path, &key, sk, buf_size, ubuf,
                                 &sk_offset, &num_found);
                btrfs_release_path(path);
                if (ret)
                        break;

        }
and in copy_to_sk() -
                sh.objectid = key->objectid;
                sh.offset = key->offset;
                sh.type = key->type;
                sh.len = item_len;
                sh.transid = found_transid;

                /*
                 * Copy search result header. If we fault then loop again so we
                 * can fault in the pages and -EFAULT there if there's a
                 * problem. Otherwise we'll fault and then copy the buffer in
                 * properly this next time through
                 */
                if (copy_to_user_nofault(ubuf + *sk_offset, &sh, sizeof(sh))) {
                        ret = 0;
                        goto out;
                }
with sk_offset left unchanged if the very first copy_to_user_nofault() fails.

Now, consider a situation on arm64 where ubuf points to the beginning of page,
ubuf[0] can be accessed, but ubuf[16] can not (possible with MTE, AFAICS).  We do
fault_in_pages_writeable(), which succeeds.  When we get to copy_to_user_nofault()
we fail as soon as it gets past the first 16 bytes.  And we repeat everything from
scratch, with no progress made, since short copies are treated as "discard and
repeat" here.

Am I misreading what's going on there?