Skip to content

Commit

Permalink
ocfs2: do not write error flag to user structure we cannot copy from/to
Browse files Browse the repository at this point in the history 
If we failed to copy from the structure, writing back the flags leaks 31
bits of kernel memory (the rest of the ir_flags field).

In any case, if we cannot copy from/to the structure, why should we
expect putting just the flags to work?

Also make sure ocfs2_info_handle_freeinode() returns the right error
code if the copy_to_user() fails.

Fixes: ddee5cd ('Ocfs2: Add new OCFS2_IOC_INFO ioctl for ocfs2 v8.')
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Joel Becker <jlbec@evilplan.org>
Acked-by: Mark Fasheh <mfasheh@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Ben Hutchings authored and Linus Torvalds committed Aug 29, 2014
1 parent 4df4185 commit 2b46263
Showing 1 changed file with 43 additions and 86 deletions.
129 changes: 43 additions & 86 deletions fs/ocfs2/ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@
copy_to_user((typeof(a) __user *)b, &(a), sizeof(a))

/*
* This call is void because we are already reporting an error that may
* be -EFAULT. The error will be returned from the ioctl(2) call. It's
* just a best-effort to tell userspace that this request caused the error.
* This is just a best-effort to tell userspace that this request
* caused the error.
*/
static inline void o2info_set_request_error(struct ocfs2_info_request *kreq,
struct ocfs2_info_request __user *req)
Expand Down Expand Up @@ -146,136 +145,105 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
static int ocfs2_info_handle_blocksize(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_blocksize oib;

if (o2info_from_user(oib, req))
goto bail;
return -EFAULT;

oib.ib_blocksize = inode->i_sb->s_blocksize;

o2info_set_request_filled(&oib.ib_req);

if (o2info_to_user(oib, req))
goto bail;

status = 0;
bail:
if (status)
o2info_set_request_error(&oib.ib_req, req);
return -EFAULT;

return status;
return 0;
}

static int ocfs2_info_handle_clustersize(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_clustersize oic;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oic, req))
goto bail;
return -EFAULT;

oic.ic_clustersize = osb->s_clustersize;

o2info_set_request_filled(&oic.ic_req);

if (o2info_to_user(oic, req))
goto bail;

status = 0;
bail:
if (status)
o2info_set_request_error(&oic.ic_req, req);
return -EFAULT;

return status;
return 0;
}

static int ocfs2_info_handle_maxslots(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_maxslots oim;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oim, req))
goto bail;
return -EFAULT;

oim.im_max_slots = osb->max_slots;

o2info_set_request_filled(&oim.im_req);

if (o2info_to_user(oim, req))
goto bail;
return -EFAULT;

status = 0;
bail:
if (status)
o2info_set_request_error(&oim.im_req, req);

return status;
return 0;
}

static int ocfs2_info_handle_label(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_label oil;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oil, req))
goto bail;
return -EFAULT;

memcpy(oil.il_label, osb->vol_label, OCFS2_MAX_VOL_LABEL_LEN);

o2info_set_request_filled(&oil.il_req);

if (o2info_to_user(oil, req))
goto bail;
return -EFAULT;

status = 0;
bail:
if (status)
o2info_set_request_error(&oil.il_req, req);

return status;
return 0;
}

static int ocfs2_info_handle_uuid(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_uuid oiu;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oiu, req))
goto bail;
return -EFAULT;

memcpy(oiu.iu_uuid_str, osb->uuid_str, OCFS2_TEXT_UUID_LEN + 1);

o2info_set_request_filled(&oiu.iu_req);

if (o2info_to_user(oiu, req))
goto bail;

status = 0;
bail:
if (status)
o2info_set_request_error(&oiu.iu_req, req);
return -EFAULT;

return status;
return 0;
}

static int ocfs2_info_handle_fs_features(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_fs_features oif;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oif, req))
goto bail;
return -EFAULT;

oif.if_compat_features = osb->s_feature_compat;
oif.if_incompat_features = osb->s_feature_incompat;
Expand All @@ -284,39 +252,28 @@ static int ocfs2_info_handle_fs_features(struct inode *inode,
o2info_set_request_filled(&oif.if_req);

if (o2info_to_user(oif, req))
goto bail;
return -EFAULT;

status = 0;
bail:
if (status)
o2info_set_request_error(&oif.if_req, req);

return status;
return 0;
}

static int ocfs2_info_handle_journal_size(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_journal_size oij;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

if (o2info_from_user(oij, req))
goto bail;
return -EFAULT;

oij.ij_journal_size = i_size_read(osb->journal->j_inode);

o2info_set_request_filled(&oij.ij_req);

if (o2info_to_user(oij, req))
goto bail;
return -EFAULT;

status = 0;
bail:
if (status)
o2info_set_request_error(&oij.ij_req, req);

return status;
return 0;
}

static int ocfs2_info_scan_inode_alloc(struct ocfs2_super *osb,
Expand Down Expand Up @@ -373,7 +330,7 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
u32 i;
u64 blkno = -1;
char namebuf[40];
int status = -EFAULT, type = INODE_ALLOC_SYSTEM_INODE;
int status, type = INODE_ALLOC_SYSTEM_INODE;
struct ocfs2_info_freeinode *oifi = NULL;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
struct inode *inode_alloc = NULL;
Expand All @@ -385,8 +342,10 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
goto out_err;
}

if (o2info_from_user(*oifi, req))
goto bail;
if (o2info_from_user(*oifi, req)) {
status = -EFAULT;
goto out_free;
}

oifi->ifi_slotnum = osb->max_slots;

Expand Down Expand Up @@ -424,14 +383,16 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,

o2info_set_request_filled(&oifi->ifi_req);

if (o2info_to_user(*oifi, req))
goto bail;
if (o2info_to_user(*oifi, req)) {
status = -EFAULT;
goto out_free;
}

status = 0;
bail:
if (status)
o2info_set_request_error(&oifi->ifi_req, req);

out_free:
kfree(oifi);
out_err:
return status;
Expand Down Expand Up @@ -658,7 +619,7 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
{
u64 blkno = -1;
char namebuf[40];
int status = -EFAULT, type = GLOBAL_BITMAP_SYSTEM_INODE;
int status, type = GLOBAL_BITMAP_SYSTEM_INODE;

struct ocfs2_info_freefrag *oiff;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
Expand All @@ -671,8 +632,10 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
goto out_err;
}

if (o2info_from_user(*oiff, req))
goto bail;
if (o2info_from_user(*oiff, req)) {
status = -EFAULT;
goto out_free;
}
/*
* chunksize from userspace should be power of 2.
*/
Expand Down Expand Up @@ -711,14 +674,14 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,

if (o2info_to_user(*oiff, req)) {
status = -EFAULT;
goto bail;
goto out_free;
}

status = 0;
bail:
if (status)
o2info_set_request_error(&oiff->iff_req, req);

out_free:
kfree(oiff);
out_err:
return status;
Expand All @@ -727,23 +690,17 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
static int ocfs2_info_handle_unknown(struct inode *inode,
struct ocfs2_info_request __user *req)
{
int status = -EFAULT;
struct ocfs2_info_request oir;

if (o2info_from_user(oir, req))
goto bail;
return -EFAULT;

o2info_clear_request_filled(&oir);

if (o2info_to_user(oir, req))
goto bail;
return -EFAULT;

status = 0;
bail:
if (status)
o2info_set_request_error(&oir, req);

return status;
return 0;
}

/*
Expand Down

0 comments on commit 2b46263

Please sign in to comment.