From: Matthew Wilcox <willy@infradead.org>
To: Dan Carpenter <dan.carpenter@oracle.com>,
	Julia Lawall <julia.lawall@inria.fr>
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Viacheslav Dubeyko <slava@dubeyko.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Bart Van Assche <bvanassche@acm.org>,
	Kees Cook <keescook@chromium.org>,
	linux-fsdevel@vger.kernel.org
Subject: kmap + memmove
Date: Sun, 5 May 2024 13:25:58 +0100	[thread overview]
Message-ID: <Zjd61vTCQoDN9tUJ@casper.infradead.org> (raw)
Here's a fun bug that's not obvious:
hfs_bnode_move:
                                dst_ptr = kmap_local_page(*dst_page);
                                src_ptr = kmap_local_page(*src_page);
                                memmove(dst_ptr, src_ptr, src);
If both of the pointers are guaranteed to come from diffeerent calls to
kmap_local(), memmove() is probably not going to do what you want.
Worth a smatch or coccinelle rule?
The only time that memmove() is going to do something different from
memcpy() is when src and dst overlap.  But if src and dst both come
from kmap_local(), they're guaranteed to not overlap.  Even if dst_page
and src_page were the same.
Which means the conversion in 6c3014a67a44 was buggy.  Calling kmap()
for the same page twice gives you the same address.  Calling kmap_local()
for the same page twice gives you two different addresses.
Fabio, how many other times did you create this same bug?  Ira, I'm
surprised you didn't catch this one; you created the same bug in
memmove_page() which I got Fabio to delete in 9384d79249d0.
next             reply	other threads:[~2024-05-05 12:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-05 12:25 Matthew Wilcox [this message]
2024-05-05 13:01 ` kmap + memmove Julia Lawall
2024-05-06  3:40   ` Ira Weiny
2024-05-06  5:15     ` Julia Lawall
2024-05-06  5:48     ` Julia Lawall
2024-05-06  5:50       ` Julia Lawall
2024-05-06  3:47   ` Matthew Wilcox
2024-05-06  4:14 ` Matthew Wilcox
2024-05-24 19:35   ` Matthew Wilcox
2024-08-22 18:54     ` Matthew Wilcox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=Zjd61vTCQoDN9tUJ@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=bvanassche@acm.org \
    --cc=dan.carpenter@oracle.com \
    --cc=fmdefrancesco@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=julia.lawall@inria.fr \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).