linux-kernel - Re: [PATCH v2 2/3] fs/ufs: Change the signature of ufs_get

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]

Message-ID: <Y5glgpD7fFifC4Fi@ZenIV>
Date:   Tue, 13 Dec 2022 07:10:58 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc:     Evgeniy Dushistov <dushistov@...l.ru>,
        Ira Weiny <ira.weiny@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] fs/ufs: Change the signature of ufs_get_page()

On Tue, Dec 13, 2022 at 12:19:05AM +0100, Fabio M. De Francesco wrote:
> +static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **page)
>  {
>  	struct address_space *mapping = dir->i_mapping;
> -	struct page *page = read_mapping_page(mapping, n, NULL);
> -	if (!IS_ERR(page)) {
> -		kmap(page);
> -		if (unlikely(!PageChecked(page))) {
> -			if (!ufs_check_page(page))
> +	*page = read_mapping_page(mapping, n, NULL);
> +	if (!IS_ERR(*page)) {
> +		kmap(*page);
> +		if (unlikely(!PageChecked(*page))) {
> +			if (!ufs_check_page(*page))
>  				goto fail;
>  		}
>  	}
> -	return page;
> +	return page_address(*page);

Er...   You really don't want to do that when you've got ERR_PTR()
from read_mapping_page().
>  
>  fail:
> -	ufs_put_page(page);
> +	ufs_put_page(*page);
>  	return ERR_PTR(-EIO);
>  }

IDGI...

static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **p)
{
	struct address_space *mapping = dir->i_mapping;
	struct page *page = read_mapping_page(mapping, n, NULL);

	if (!IS_ERR(page)) {
		kmap(page);
		if (unlikely(!PageChecked(page))) {
			if (!ufs_check_page(page))
				goto fail;
		}
		*p = page;
		return page_address(page);
	}
	return ERR_CAST(page);

fail:
	ufs_put_page(page);
	return ERR_PTR(-EIO);
}

all there is to it...  The only things you need to change are
1) type of function
2) make sure to store the page into that pointer to pointer to page on success
3) return page_address(page) instead of page on success
4) use ERR_CAST() to convert ERR_PTR() that is struct page * into equal
ERR_PTR() that is void * (the last one is optional, just makes the intent
more clear).