linux-kernel - RE: [PATCH] Propagate CRAMFS uncompression errors

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: <FF038EB85946AA46B18DFEE6E6F8A289950C30@xmb-rtp-218.amer.cisco.com>
Date:	Mon, 9 Feb 2009 22:28:15 -0500
From:	"David VomLehn (dvomlehn)" <dvomlehn@...co.com>
To:	"Andrew Morton" <akpm@...ux-foundation.org>
Cc:	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] Propagate CRAMFS uncompression errors


> From: Andrew Morton [mailto:akpm@...ux-foundation.org] 
> Sent: Monday, February 09, 2009 3:17 PM
> Subject: Re: [PATCH] Propagate CRAMFS uncompression errors
> 
> "David VomLehn (dvomlehn)" <dvomlehn@...co.com> wrote:
> 
> > If cramfs_uncompress_block detects an error uncompressing it will
> > return a zero value. This patch checks the return value and 
> propagates
> > the error back up to the block layer.
> > 
> > Signed-off-by: David VomLehn <dvomlehn@...co.com>
> > ---
> >  fs/cramfs/inode.c |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> > index a07338d..6ff8a5e 100644
> > --- a/fs/cramfs/inode.c
> > +++ b/fs/cramfs/inode.c
> > @@ -493,7 +493,15 @@ static int cramfs_readpage(struct file *file,
> > struct page * page)
> 
> Your email client is wordwrapping the patches.

Sorry. I sent it to myself first, and didn't have a problem. I hate
email programs.

> >  	memset(pgdata + bytes_filled, 0, PAGE_CACHE_SIZE -
> > bytes_filled);
> >  	kunmap(page);
> >  	flush_dcache_page(page);
> > -	SetPageUptodate(page);
> > +
> > +	if (bytes_filled == 0) {
> > +		ClearPageUptodate(page);
> > +		SetPageError(page);
> > +	}
> > +
> > +	else
> > +		SetPageUptodate(page);
> > +
> >  	unlock_page(page);
> >  	return 0;
> 
> A more typical code layout would be
> 
> 	if (bytes_filled == 0) {
> 		ClearPageUptodate(page);
> 		SetPageError(page);
> 	} else
> 		SetPageUptodate(page);
> 
> or (better, IMO):
> 
> 	if (bytes_filled == 0) {
> 		ClearPageUptodate(page);
> 		SetPageError(page);
> 	} else {
> 		SetPageUptodate(page);
> 	}
> 
> 
> This patch will incorrectly cause the driver to report an IO error if
> the (page->index < maxblock) test returns false.  For example, a
> pread() which is wholly outside the end-of-file should return 
> zero, not
> -EIO.
> 
> cramfs_readpage() handles this case very strangely, although not
> obviously buggily.  Probably this function never even gets 
> called for a
> read wholly outside i_size.
> 
> How does this version look to you?
> 
> --- a/fs/cramfs/inode.c~propagate-cramfs-uncompression-errors
> +++ a/fs/cramfs/inode.c
> @@ -488,12 +488,19 @@ static int cramfs_readpage(struct file *
>  				 compr_len);
>  			mutex_unlock(&read_mutex);
>  		}
> -	} else
> -		pgdata = kmap(page);
> -	memset(pgdata + bytes_filled, 0, PAGE_CACHE_SIZE - 
> bytes_filled);
> -	kunmap(page);
> -	flush_dcache_page(page);
> -	SetPageUptodate(page);
> +
> +		if (bytes_filled == 0) {
> +			/* Decompression error */
> +			ClearPageUptodate(page);
> +			SetPageError(page);
> +		} else {
> +			memset(pgdata + bytes_filled, 0,
> +					PAGE_CACHE_SIZE - bytes_filled);
> +			flush_dcache_page(page);
> +			SetPageUptodate(page);
> +		}
> +		kunmap(page);
> +	}
>  	unlock_page(page);
>  	return 0;
>  }

Better, thanks!
--
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/