Re: [PATCH] jfs: Fix buffer overrun in ea_get

[Dave Kleikamp]

On 06/18/2018 03:38 AM, Nikolay Borisov wrote:
> Currently ea_buf->xattr buffer is allocated with min(min_size, ea_size).
> This is wrong since after the xattr buffer is allocated the ->max_size
> variable is actually rounded up to th next ->s_blocksize size. Fix this
> by using the rounded up max_size as input to the malloc.
> 
> Suggested-by: Shankara Pailoor <shankarapailoor@xxxxxxxxx>
> Reported-by: Shankara Pailoor <shankarapailoor@xxxxxxxxx>
> CC: shankarapailoor@xxxxxxxxx
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> Hello David, 
> 
> I'm sending you the patch for the issue which was originally reported and 
> suggested by Shankar.  I won't usually got and override the original 
> author of a patch but given the clear lack of experience with upstream (missing 
> SOB line, no changelog explaining the change etc) and the 
> fact there is already a CVE for this issue (using syzkaller for quick CVE 
> generation seems to be all the rage these days, go figure...) I'd rather have 
> an upstream, backportable version sooner rather than later. 

Thanks, but I already had Shankar's patch in the linux-next build. I was
out on vacation so I didn't get it pushed to Linus as soon as I would
have liked, but did so this morning.

I appreciate your effort.

Shaggy

> 
>  fs/jfs/xattr.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
> index c60f3d32ee91..96b9355ff69a 100644
> --- a/fs/jfs/xattr.c
> +++ b/fs/jfs/xattr.c
> @@ -493,14 +493,14 @@ static int ea_get(struct inode *inode, struct ea_buffer *ea_buf, int min_size)
>  		 * To keep the rest of the code simple.  Allocate a
>  		 * contiguous buffer to work with
>  		 */
> -		ea_buf->xattr = kmalloc(size, GFP_KERNEL);
> -		if (ea_buf->xattr == NULL)
> -			return -ENOMEM;
> -
>  		ea_buf->flag = EA_MALLOC;
>  		ea_buf->max_size = (size + sb->s_blocksize - 1) &
>  		    ~(sb->s_blocksize - 1);
>  
> +		ea_buf->xattr = kmalloc(ea_buf->max_size, GFP_KERNEL);
> +		if (ea_buf->xattr == NULL)
> +			return -ENOMEM;
> +
>  		if (ea_size == 0)
>  			return 0;
>  
>