public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] xfs: underflow bug in xfs_attrlist_by_handle()
       [not found] <20131025144452.GA28451@ngolde.de>
@ 2013-10-31 18:00 ` Dan Carpenter
  2013-10-31 21:34   ` Dave Chinner
  2013-12-04 21:53   ` Ben Myers
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-10-31 18:00 UTC (permalink / raw)
  To: Ben Myers; +Cc: Fabian Yamaguchi, security, Alex Elder, Nico Golde, xfs

If we allocate less than sizeof(struct attrlist) then we end up
corrupting memory or doing a ZERO_PTR_SIZE dereference.

This can only be triggered with CAP_SYS_ADMIN.

Reported-by: Nico Golde <nico@ngolde.de>
Reported-by: Fabian Yamaguchi <fabs@goesec.de>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 4d61340..33ad9a7 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -442,7 +442,8 @@ xfs_attrlist_by_handle(
 		return -XFS_ERROR(EPERM);
 	if (copy_from_user(&al_hreq, arg, sizeof(xfs_fsop_attrlist_handlereq_t)))
 		return -XFS_ERROR(EFAULT);
-	if (al_hreq.buflen > XATTR_LIST_MAX)
+	if (al_hreq.buflen < sizeof(struct attrlist) ||
+	    al_hreq.buflen > XATTR_LIST_MAX)
 		return -XFS_ERROR(EINVAL);
 
 	/*
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index e8fb123..a7992f8 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -356,7 +356,8 @@ xfs_compat_attrlist_by_handle(
 	if (copy_from_user(&al_hreq, arg,
 			   sizeof(compat_xfs_fsop_attrlist_handlereq_t)))
 		return -XFS_ERROR(EFAULT);
-	if (al_hreq.buflen > XATTR_LIST_MAX)
+	if (al_hreq.buflen < sizeof(struct attrlist) ||
+	    al_hreq.buflen > XATTR_LIST_MAX)
 		return -XFS_ERROR(EINVAL);
 
 	/*

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [patch] xfs: underflow bug in xfs_attrlist_by_handle()
  2013-10-31 18:00 ` [patch] xfs: underflow bug in xfs_attrlist_by_handle() Dan Carpenter
@ 2013-10-31 21:34   ` Dave Chinner
  2013-12-04 21:53   ` Ben Myers
  1 sibling, 0 replies; 3+ messages in thread
From: Dave Chinner @ 2013-10-31 21:34 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Fabian Yamaguchi, security, Alex Elder, xfs, Ben Myers,
	Nico Golde

On Thu, Oct 31, 2013 at 09:00:10PM +0300, Dan Carpenter wrote:
> If we allocate less than sizeof(struct attrlist) then we end up
> corrupting memory or doing a ZERO_PTR_SIZE dereference.
> 
> This can only be triggered with CAP_SYS_ADMIN.
> 
> Reported-by: Nico Golde <nico@ngolde.de>
> Reported-by: Fabian Yamaguchi <fabs@goesec.de>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 4d61340..33ad9a7 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -442,7 +442,8 @@ xfs_attrlist_by_handle(
>  		return -XFS_ERROR(EPERM);
>  	if (copy_from_user(&al_hreq, arg, sizeof(xfs_fsop_attrlist_handlereq_t)))
>  		return -XFS_ERROR(EFAULT);
> -	if (al_hreq.buflen > XATTR_LIST_MAX)
> +	if (al_hreq.buflen < sizeof(struct attrlist) ||
> +	    al_hreq.buflen > XATTR_LIST_MAX)
>  		return -XFS_ERROR(EINVAL);

Yup, that's not checked in xfs_attr_list(). Looks like these are the
only direct callers of xfs_attr_list(), and the other callers of
xfs_attr_list_int() don't appear to have the same issue. Good find!

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [patch] xfs: underflow bug in xfs_attrlist_by_handle()
  2013-10-31 18:00 ` [patch] xfs: underflow bug in xfs_attrlist_by_handle() Dan Carpenter
  2013-10-31 21:34   ` Dave Chinner
@ 2013-12-04 21:53   ` Ben Myers
  1 sibling, 0 replies; 3+ messages in thread
From: Ben Myers @ 2013-12-04 21:53 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Fabian Yamaguchi, security, Alex Elder, Nico Golde, xfs

On Thu, Oct 31, 2013 at 09:00:10PM +0300, Dan Carpenter wrote:
> If we allocate less than sizeof(struct attrlist) then we end up
> corrupting memory or doing a ZERO_PTR_SIZE dereference.
> 
> This can only be triggered with CAP_SYS_ADMIN.
> 
> Reported-by: Nico Golde <nico@ngolde.de>
> Reported-by: Fabian Yamaguchi <fabs@goesec.de>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied.  Thanks Dan.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-12-04 21:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20131025144452.GA28451@ngolde.de>
2013-10-31 18:00 ` [patch] xfs: underflow bug in xfs_attrlist_by_handle() Dan Carpenter
2013-10-31 21:34   ` Dave Chinner
2013-12-04 21:53   ` Ben Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox