Re: [PATCH] swap: divide-by-zero when zero length swap file on ssd

2018-04-06 Thread Andrew Morton
On Fri, 6 Apr 2018 09:52:50 -0700 Randy Dunlap  wrote:

> [adding linux-mm and akpm]

Thanks.

> ...

The patch is a huge mess, with leading and trailing whitespace.  I
fixed all that up, but we'd like to receive Tom's signed-off-by:, please. 
Documentation/process/submitting-patches.rst section 11 has the
details.



Re: [PATCH] swap: divide-by-zero when zero length swap file on ssd

2018-04-06 Thread Randy Dunlap
[adding linux-mm and akpm]

On 04/06/2018 07:11 AM, Tom Abraham wrote:
>   
>   
> Calling swapon() on a zero length swap file on SSD can lead to a  
>   
> divide-by-zero.   
>   
>   
>   
> Although creating such files isn't possible with mkswap and they woud be  
>   
> considered invalid, it would be better for the swapon code to be more robust  
>   
> and handle this condition gracefully (return -EINVAL). Especially since the 
> fix 
> is small and straight-forward.
>   
>   
>   
> To help with wear leveling on SSD, the swapon syscall calculates a random 
>   
> position in the swap file using modulo p->highest_bit, which is set to
>   
> maxpages - 1 in read_swap_header. 
>   
>   
>   
> If the swap file is zero length, read_swap_header sets maxpages=1 and 
>   
> last_page=0, resulting in p->highest_bit=0 and we divide-by-zero when we 
> modulo 
> p->highest_bit in swapon syscall. 
>   
>   
>   
> This can be prevented by having read_swap_header return zero if last_page is  
>   
> zero. 
>   
>   
>   
> diff --git a/mm/swapfile.c b/mm/swapfile.c
>   
> index c7a33717d079..d6b7bd9f365d 100644   
>   
> --- a/mm/swapfile.c   
>   
> +++ b/mm/swapfile.c   
>   
> @@ -2961,6 +2961,10 @@ static unsigned long read_swap_header(struct 
> swap_info_struct *p,
> maxpages = swp_offset(pte_to_swp_entry(   
>   
> swp_entry_to_pte(swp_entry(0, ~0UL + 1;   
>   
> last_page = swap_header->info.last_page;  
>   
> +   if(!last_page) {  
>   
> +   pr_warn("Empty swap-file\n"); 
>   
> +   return 0; 
>   
> +   } 
>   
> if (last_page > maxpages) {   
>   
> pr_warn("Truncating oversized swap area, only using %luk out 
> of %luk\n",
> maxpages << (PAGE_SHIFT - 10),
> 


-- 
~Randy