* [PATCH] xfs: fix memory exposure problems
@ 2017-04-01 1:29 Darrick J. Wong
2017-04-01 7:04 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2017-04-01 1:29 UTC (permalink / raw)
To: xfs
Fix a couple of memory exposure problems in the getbmap implementation
where we copy too much header data from userspace, and a second problem
in inumbers where we allocate an array of structures with holes, fail to
zero the holes, then blindly copy the kernel memory contents into
userspace.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_ioctl.c | 4 ++--
fs/xfs/xfs_itable.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index b0250ed..14c2301 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1547,10 +1547,10 @@ xfs_ioc_getbmap(
unsigned int cmd,
void __user *arg)
{
- struct getbmapx bmx;
+ struct getbmapx bmx = {0};
int error;
- if (copy_from_user(&bmx, arg, sizeof(struct getbmapx)))
+ if (copy_from_user(&bmx, arg, sizeof(struct getbmap)))
return -EFAULT;
if (bmx.bmv_count < 2)
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index e775f78..55642cd 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -584,7 +584,7 @@ xfs_inumbers(
return error;
bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
- buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
+ buffer = kmem_zalloc(bcount * sizeof(*buffer), KM_SLEEP);
do {
struct xfs_inobt_rec_incore r;
int stat;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: fix memory exposure problems
2017-04-01 1:29 [PATCH] xfs: fix memory exposure problems Darrick J. Wong
@ 2017-04-01 7:04 ` Christoph Hellwig
2017-04-01 18:14 ` Eric Sandeen
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2017-04-01 7:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Fri, Mar 31, 2017 at 06:29:34PM -0700, Darrick J. Wong wrote:
> Fix a couple of memory exposure problems in the getbmap implementation
> where we copy too much header data from userspace, and a second problem
> in inumbers where we allocate an array of structures with holes, fail to
> zero the holes, then blindly copy the kernel memory contents into
> userspace.
This should be two different patches.
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1547,10 +1547,10 @@ xfs_ioc_getbmap(
> unsigned int cmd,
> void __user *arg)
> {
> - struct getbmapx bmx;
> + struct getbmapx bmx = {0};
missing spaces around the zero.
> int error;
>
> - if (copy_from_user(&bmx, arg, sizeof(struct getbmapx)))
> + if (copy_from_user(&bmx, arg, sizeof(struct getbmap)))
> return -EFAULT;
This really needs a comment on how struct getbmap is a strict subset
of struct getbmapx. Or even better don't use getbmap at all
and use offsetoff on struct getbmapx.
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index e775f78..55642cd 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -584,7 +584,7 @@ xfs_inumbers(
> return error;
>
> bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
> - buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
> + buffer = kmem_zalloc(bcount * sizeof(*buffer), KM_SLEEP);
This looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: fix memory exposure problems
2017-04-01 7:04 ` Christoph Hellwig
@ 2017-04-01 18:14 ` Eric Sandeen
0 siblings, 0 replies; 3+ messages in thread
From: Eric Sandeen @ 2017-04-01 18:14 UTC (permalink / raw)
To: Christoph Hellwig, Darrick J. Wong; +Cc: xfs
On 4/1/17 2:04 AM, Christoph Hellwig wrote:
> On Fri, Mar 31, 2017 at 06:29:34PM -0700, Darrick J. Wong wrote:
>> Fix a couple of memory exposure problems in the getbmap implementation
>> where we copy too much header data from userspace, and a second problem
>> in inumbers where we allocate an array of structures with holes, fail to
>> zero the holes, then blindly copy the kernel memory contents into
>> userspace.
>
> This should be two different patches.
>
>> --- a/fs/xfs/xfs_ioctl.c
>> +++ b/fs/xfs/xfs_ioctl.c
>> @@ -1547,10 +1547,10 @@ xfs_ioc_getbmap(
>> unsigned int cmd,
>> void __user *arg)
>> {
>> - struct getbmapx bmx;
>> + struct getbmapx bmx = {0};
>
> missing spaces around the zero.
>
>> int error;
>>
>> - if (copy_from_user(&bmx, arg, sizeof(struct getbmapx)))
>> + if (copy_from_user(&bmx, arg, sizeof(struct getbmap)))
>> return -EFAULT;
>
> This really needs a comment on how struct getbmap is a strict subset
> of struct getbmapx. Or even better don't use getbmap at all
> and use offsetoff on struct getbmapx.
Agreed, was going to make the same observation.
-Eric
>> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
>> index e775f78..55642cd 100644
>> --- a/fs/xfs/xfs_itable.c
>> +++ b/fs/xfs/xfs_itable.c
>> @@ -584,7 +584,7 @@ xfs_inumbers(
>> return error;
>>
>> bcount = MIN(left, (int)(PAGE_SIZE / sizeof(*buffer)));
>> - buffer = kmem_alloc(bcount * sizeof(*buffer), KM_SLEEP);
>> + buffer = kmem_zalloc(bcount * sizeof(*buffer), KM_SLEEP);
>
> This looks fine:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-01 18:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-01 1:29 [PATCH] xfs: fix memory exposure problems Darrick J. Wong
2017-04-01 7:04 ` Christoph Hellwig
2017-04-01 18:14 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.