linux-kernel - Re: [PATCH] shmem: call set_page

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: <Pine.LNX.4.64.0907151520050.27929@sister.anvils>
Date:	Wed, 15 Jul 2009 15:22:46 +0100 (BST)
From:	Hugh Dickins <hugh.dickins@...cali.co.uk>
To:	Wu Fengguang <fengguang.wu@...el.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH] shmem: call set_page_dirty() with locked page

On Wed, 15 Jul 2009, Wu Fengguang wrote:
> shmem: call set_page_dirty() with locked page
> 
> The dirtying of page and set_page_dirty() can be moved into the page lock.
> 
> - In shmem_write_end(), the page was dirtied while the page lock was held,
>   but it's being marked dirty just after dropping the page lock.
> - In shmem_symlink(), both dirtying and marking can be moved into page lock.
> 
> It's valuable for the hwpoison code to know whether one bad page can be dropped
> without losing data. It mainly judges by testing the PG_dirty bit after taking
> the page lock. So it becomes important that the dirtying of page and the
> marking of dirtiness are both done inside the page lock. Which is a common
> practice, but sadly not a rule.
> 
> The noticeable exceptions are
> - mapped pages
> - pages with buffer_heads
> The above pages could go dirty at any time. Fortunately the hwpoison will
> unmap the page and release the buffer_heads beforehand anyway.
> 
> Many other types of pages (eg. metadata pages) can also be dirtied at will by
> their owners, the hwpoison code cannot do meaningful things to them anyway. 
> Only the dirtiness of pagecache pages owned by regular files are interested.
> 
> CC: Hugh Dickins <hugh@...itas.com>

Ah, those were the days... ;)

> Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>

Acked-by: Hugh Dickins <hugh.dickins@...cali.co.uk>

> ---
>  mm/shmem.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux.orig/mm/shmem.c
> +++ linux/mm/shmem.c
> @@ -1630,8 +1630,8 @@ shmem_write_end(struct file *file, struc
>  	if (pos + copied > inode->i_size)
>  		i_size_write(inode, pos + copied);
>  
> -	unlock_page(page);
>  	set_page_dirty(page);
> +	unlock_page(page);
>  	page_cache_release(page);
>  
>  	return copied;
> @@ -1968,13 +1968,13 @@ static int shmem_symlink(struct inode *d
>  			iput(inode);
>  			return error;
>  		}
> -		unlock_page(page);
>  		inode->i_mapping->a_ops = &shmem_aops;
>  		inode->i_op = &shmem_symlink_inode_operations;
>  		kaddr = kmap_atomic(page, KM_USER0);
>  		memcpy(kaddr, symname, len);
>  		kunmap_atomic(kaddr, KM_USER0);
>  		set_page_dirty(page);
> +		unlock_page(page);
>  		page_cache_release(page);
>  	}
>  	if (dir->i_mode & S_ISGID)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/