Re: kmap + memmove

[Matthew Wilcox <willy@infradead.org>]

On Sun, May 05, 2024 at 03:01:56PM +0200, Julia Lawall wrote:
> On Sun, 5 May 2024, Matthew Wilcox wrote:
> > 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?
> 
> I tried the following rule:
> 
> @@
> expression dst_ptr, src_ptr, dst_page, src_page, src;
> @@
> 
> *                                dst_ptr = kmap_local_page(dst_page);
> 				... when any
> *                                src_ptr = kmap_local_page(src_page);
> 				... when any
> *                                memmove(dst_ptr, src_ptr, src);
> 
> That is, basically what you wrote, but with anything in between the lines,
> and the various variables being any expression.
> 
> I only got the following results, which I guess are what you are already
> looking at:

Great, thanks!  I tried something a little more crude:

$ git grep -A10 kmap_local |grep memmove
fs/erofs/decompressor.c-				memmove(kin + rq->pageofs_out, kin + pi, cur);
fs/erofs/decompressor.c-				memmove(kin + po,
fs/hfs/bnode.c-	memmove(ptr + dst, ptr + src, len);
fs/hfsplus/bnode.c-				memmove(dst_ptr, src_ptr, src);
fs/hfsplus/bnode.c-			memmove(dst_ptr + src, src_ptr + src, len);
fs/hfsplus/bnode.c-			memmove(dst_ptr, src_ptr, l);
fs/hfsplus/bnode.c-				memmove(dst_ptr, src_ptr, l);

$ git grep -A10 kmap_atomic |grep memmove
net/sunrpc/xdr.c-			memmove(vto + pgto_base, vto + pgfrom_base, copy);
net/sunrpc/xdr.c-			memmove(vto + pgto_base, vto + pgfrom_base, copy);

All of these (other than hfsplus) are "false positives" in that you can
see the src/dst are from the same call to kmap_local/kmap_atomic.  Glad
to see there's nothing else.