On Sun, Dec 11, 2022 at 10:31:11PM +0100, Fabio M. De Francesco wrote:
> +/*
> + * Calls to ufs_get_page()/ufs_put_page() must be nested according to the
> + * rules documented in kmap_local_page()/kunmap_local().
> + *
> + * NOTE: ufs_find_entry() and ufs_dotdot() act as calls to ufs_get_page()
> + * and must be treated accordingly for nesting purposes.
> + */
> static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **page)
> {
> + char *kaddr;
> +
> struct address_space *mapping = dir->i_mapping;
> *page = read_mapping_page(mapping, n, NULL);
> if (!IS_ERR(*page)) {
> - kmap(*page);
> + kmap_local_page(*page);
> if (unlikely(!PageChecked(*page))) {
> - if (!ufs_check_page(*page))
> + if (!ufs_check_page(*page, kaddr))
Er... Building the patched tree is occasionally useful.
Here kaddr is obviously uninitialized and compiler would've
probably caught that.
And return value of kmap_local_page() is lost, which
is related to the previous issue ;-)
> goto fail;
> }
> }
> - return page;
> + return *page;
Hell, no. Callers expect the pointer to the first byte of
your page. What it should return is kaddr.
> @@ -388,7 +406,8 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode)
> mark_inode_dirty(dir);
> /* OFFSET_CACHE */
> out_put:
> - ufs_put_page(page);
> + ufs_put_page(page, kaddr);
> + return 0;
> out_unlock:
> unlock_page(page);
> goto out_put;
That can't be right. Places like
if (err)
goto out_unlock;
do not expect err to be lost. You end up returning 0 now. Something strange
happened here (in the previous commit, perhaps?)
