Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] jfs: Fix buffer overrun in ea_get
To:     Nikolay Borisov <nborisov@suse.com>
Cc:     jfs-discussion@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        shankarapailoor@gmail.com
References: <1529311091-8307-1-git-send-email-nborisov@suse.com>
From:   Dave Kleikamp <dave.kleikamp@oracle.com>
Openpgp: preference=signencrypt
Message-ID: <245a3ccb-3390-d650-91f0-c8e6f002499f@oracle.com>
Date:   Mon, 18 Jun 2018 08:10:23 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <1529311091-8307-1-git-send-email-nborisov@suse.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@gmail.com>
> Reported-by: Shankara Pailoor <shankarapailoor@gmail.com>
> CC: shankarapailoor@gmail.com
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 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;
>  
>