Re: kmap + memmove

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Sun, May 05, 2024 at 01:25:58PM +0100, 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);

OK, so now we know this is the only place with this problem, how are we
going to fix it?

I think the obvious thing to do is to revert the kmap -> kmap_local
conversion in this function.  The other functions look fine.

Longer term, hfs_bnode_move() makes my eyes bleed.  I really think we
need to do something stupider.  Something like ...

void hfs_bnode_move(struct hfs_bnode *node, int dst, int src, int len)
{
	void *data;
	int first, last;

	if (!len || src == dst)
		return;
	if (src < dst && src + len < dst)
		return hfs_bnode_copy(node, dst, node, src, len);
	if (dst < src && dst + len < src)
		return hfs_bnode_copy(node, dst, node, src, len);

	src += node->page_offset;
	dst += node->page_offset;
	first = min(dst, src) / PAGE_SIZE;
	last = max(dst + len, src + len) / PAGE_SIZE;
	data = vmap_folios(bnode->folios + first, last - first + 1);
	src -= first * PAGE_SIZE;
	dst -= first * PAGE_SIZE;
// maybe an off-by-one in above calculations; check it
	memmove(data + dst, data + src, len);
	vunmap(data);
}