linux-kernel - Re: [RFC PATCH 1/2] mm: swap: check if swap backing device is congested or not

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50e14a30-97bb-06c6-ae6a-74e6dc827713@linux.intel.com>
Date:   Wed, 19 Dec 2018 11:00:27 -0800
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Yang Shi <yang.shi@...ux.alibaba.com>, ying.huang@...el.com,
        tim.c.chen@...el.com, minchan@...nel.org,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] mm: swap: check if swap backing device is
 congested or not

On 12/19/18 10:40 AM, Yang Shi wrote:
> 
> 

>>>> I don't think your dereference inode = si->swap_file->f_mapping->host
>>>> is always safe.  You should do it only when (si->flags & SWP_FS) is true.
>>> Do you mean it is not safe for swap partition?
>> The f_mapping may not be instantiated.  It is only done for SWP_FS.
> 
> Really? I saw the below calls in swapon:
> 
> swap_file = file_open_name(name, O_RDWR|O_LARGEFILE, 0);
> ...
> p->swap_file = swap_file;
> mapping = swap_file->f_mapping;
> inode = mapping->host;
> ...
> 
> Then the below code manipulates the inode.
> 
> And, trace shows file_open_name() does call blkdev_open if it is turning block device swap on. And, blkdev_open() would return instantiated address_space and inode.
> 
> Am I missing something?
> 

I was trying to limit the congestion logic for block devices backed swap.
So the check I had in mind should really be "si->flags & SWP_BLKDEV"
instead of si->flags & SWP_FS.  I was concerned that there could
be other use cases where the inode dereference is invalid.

Looking at the code a bit more, looks like swap_cluster_readahead is not
used for other special case swap usage (like page migration).  So
you would a proper swapfile and inode here.  But I think it is still
a good idea to have a check for SWP_BLKDEV in si->flags.

Thanks.

Tim