Commit 2b462638 authored by Ben Hutchings's avatar Ben Hutchings Committed by Linus Torvalds

ocfs2: do not write error flag to user structure we cannot copy from/to

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: ddee5cdb ('Ocfs2: Add new OCFS2_IOC_INFO ioctl for ocfs2 v8.')
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
Cc: Joel Becker <jlbec@evilplan.org>
Acked-by: default avatarMark Fasheh <mfasheh@suse.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 4df4185a
...@@ -35,9 +35,8 @@ ...@@ -35,9 +35,8 @@
copy_to_user((typeof(a) __user *)b, &(a), sizeof(a)) copy_to_user((typeof(a) __user *)b, &(a), sizeof(a))
/* /*
* This call is void because we are already reporting an error that may * This is just a best-effort to tell userspace that this request
* be -EFAULT. The error will be returned from the ioctl(2) call. It's * caused the error.
* 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, static inline void o2info_set_request_error(struct ocfs2_info_request *kreq,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
...@@ -146,136 +145,105 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags, ...@@ -146,136 +145,105 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
static int ocfs2_info_handle_blocksize(struct inode *inode, static int ocfs2_info_handle_blocksize(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_blocksize oib; struct ocfs2_info_blocksize oib;
if (o2info_from_user(oib, req)) if (o2info_from_user(oib, req))
goto bail; return -EFAULT;
oib.ib_blocksize = inode->i_sb->s_blocksize; oib.ib_blocksize = inode->i_sb->s_blocksize;
o2info_set_request_filled(&oib.ib_req); o2info_set_request_filled(&oib.ib_req);
if (o2info_to_user(oib, req)) if (o2info_to_user(oib, req))
goto bail; return -EFAULT;
status = 0;
bail:
if (status)
o2info_set_request_error(&oib.ib_req, req);
return status; return 0;
} }
static int ocfs2_info_handle_clustersize(struct inode *inode, static int ocfs2_info_handle_clustersize(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_clustersize oic; struct ocfs2_info_clustersize oic;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oic, req)) if (o2info_from_user(oic, req))
goto bail; return -EFAULT;
oic.ic_clustersize = osb->s_clustersize; oic.ic_clustersize = osb->s_clustersize;
o2info_set_request_filled(&oic.ic_req); o2info_set_request_filled(&oic.ic_req);
if (o2info_to_user(oic, req)) if (o2info_to_user(oic, req))
goto bail; return -EFAULT;
status = 0;
bail:
if (status)
o2info_set_request_error(&oic.ic_req, req);
return status; return 0;
} }
static int ocfs2_info_handle_maxslots(struct inode *inode, static int ocfs2_info_handle_maxslots(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_maxslots oim; struct ocfs2_info_maxslots oim;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oim, req)) if (o2info_from_user(oim, req))
goto bail; return -EFAULT;
oim.im_max_slots = osb->max_slots; oim.im_max_slots = osb->max_slots;
o2info_set_request_filled(&oim.im_req); o2info_set_request_filled(&oim.im_req);
if (o2info_to_user(oim, req)) if (o2info_to_user(oim, req))
goto bail; return -EFAULT;
status = 0; return 0;
bail:
if (status)
o2info_set_request_error(&oim.im_req, req);
return status;
} }
static int ocfs2_info_handle_label(struct inode *inode, static int ocfs2_info_handle_label(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_label oil; struct ocfs2_info_label oil;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oil, req)) if (o2info_from_user(oil, req))
goto bail; return -EFAULT;
memcpy(oil.il_label, osb->vol_label, OCFS2_MAX_VOL_LABEL_LEN); memcpy(oil.il_label, osb->vol_label, OCFS2_MAX_VOL_LABEL_LEN);
o2info_set_request_filled(&oil.il_req); o2info_set_request_filled(&oil.il_req);
if (o2info_to_user(oil, req)) if (o2info_to_user(oil, req))
goto bail; return -EFAULT;
status = 0; return 0;
bail:
if (status)
o2info_set_request_error(&oil.il_req, req);
return status;
} }
static int ocfs2_info_handle_uuid(struct inode *inode, static int ocfs2_info_handle_uuid(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_uuid oiu; struct ocfs2_info_uuid oiu;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oiu, req)) if (o2info_from_user(oiu, req))
goto bail; return -EFAULT;
memcpy(oiu.iu_uuid_str, osb->uuid_str, OCFS2_TEXT_UUID_LEN + 1); memcpy(oiu.iu_uuid_str, osb->uuid_str, OCFS2_TEXT_UUID_LEN + 1);
o2info_set_request_filled(&oiu.iu_req); o2info_set_request_filled(&oiu.iu_req);
if (o2info_to_user(oiu, req)) if (o2info_to_user(oiu, req))
goto bail; return -EFAULT;
status = 0;
bail:
if (status)
o2info_set_request_error(&oiu.iu_req, req);
return status; return 0;
} }
static int ocfs2_info_handle_fs_features(struct inode *inode, static int ocfs2_info_handle_fs_features(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_fs_features oif; struct ocfs2_info_fs_features oif;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oif, req)) if (o2info_from_user(oif, req))
goto bail; return -EFAULT;
oif.if_compat_features = osb->s_feature_compat; oif.if_compat_features = osb->s_feature_compat;
oif.if_incompat_features = osb->s_feature_incompat; oif.if_incompat_features = osb->s_feature_incompat;
...@@ -284,39 +252,28 @@ static int ocfs2_info_handle_fs_features(struct inode *inode, ...@@ -284,39 +252,28 @@ static int ocfs2_info_handle_fs_features(struct inode *inode,
o2info_set_request_filled(&oif.if_req); o2info_set_request_filled(&oif.if_req);
if (o2info_to_user(oif, req)) if (o2info_to_user(oif, req))
goto bail; return -EFAULT;
status = 0; return 0;
bail:
if (status)
o2info_set_request_error(&oif.if_req, req);
return status;
} }
static int ocfs2_info_handle_journal_size(struct inode *inode, static int ocfs2_info_handle_journal_size(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_journal_size oij; struct ocfs2_info_journal_size oij;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
if (o2info_from_user(oij, req)) if (o2info_from_user(oij, req))
goto bail; return -EFAULT;
oij.ij_journal_size = i_size_read(osb->journal->j_inode); oij.ij_journal_size = i_size_read(osb->journal->j_inode);
o2info_set_request_filled(&oij.ij_req); o2info_set_request_filled(&oij.ij_req);
if (o2info_to_user(oij, req)) if (o2info_to_user(oij, req))
goto bail; return -EFAULT;
status = 0; return 0;
bail:
if (status)
o2info_set_request_error(&oij.ij_req, req);
return status;
} }
static int ocfs2_info_scan_inode_alloc(struct ocfs2_super *osb, static int ocfs2_info_scan_inode_alloc(struct ocfs2_super *osb,
...@@ -373,7 +330,7 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, ...@@ -373,7 +330,7 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
u32 i; u32 i;
u64 blkno = -1; u64 blkno = -1;
char namebuf[40]; 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_info_freeinode *oifi = NULL;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
struct inode *inode_alloc = NULL; struct inode *inode_alloc = NULL;
...@@ -385,8 +342,10 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, ...@@ -385,8 +342,10 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
goto out_err; goto out_err;
} }
if (o2info_from_user(*oifi, req)) if (o2info_from_user(*oifi, req)) {
goto bail; status = -EFAULT;
goto out_free;
}
oifi->ifi_slotnum = osb->max_slots; oifi->ifi_slotnum = osb->max_slots;
...@@ -424,14 +383,16 @@ static int ocfs2_info_handle_freeinode(struct inode *inode, ...@@ -424,14 +383,16 @@ static int ocfs2_info_handle_freeinode(struct inode *inode,
o2info_set_request_filled(&oifi->ifi_req); o2info_set_request_filled(&oifi->ifi_req);
if (o2info_to_user(*oifi, req)) if (o2info_to_user(*oifi, req)) {
goto bail; status = -EFAULT;
goto out_free;
}
status = 0; status = 0;
bail: bail:
if (status) if (status)
o2info_set_request_error(&oifi->ifi_req, req); o2info_set_request_error(&oifi->ifi_req, req);
out_free:
kfree(oifi); kfree(oifi);
out_err: out_err:
return status; return status;
...@@ -658,7 +619,7 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, ...@@ -658,7 +619,7 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
{ {
u64 blkno = -1; u64 blkno = -1;
char namebuf[40]; 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_info_freefrag *oiff;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
...@@ -671,8 +632,10 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, ...@@ -671,8 +632,10 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
goto out_err; goto out_err;
} }
if (o2info_from_user(*oiff, req)) if (o2info_from_user(*oiff, req)) {
goto bail; status = -EFAULT;
goto out_free;
}
/* /*
* chunksize from userspace should be power of 2. * chunksize from userspace should be power of 2.
*/ */
...@@ -711,14 +674,14 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, ...@@ -711,14 +674,14 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
if (o2info_to_user(*oiff, req)) { if (o2info_to_user(*oiff, req)) {
status = -EFAULT; status = -EFAULT;
goto bail; goto out_free;
} }
status = 0; status = 0;
bail: bail:
if (status) if (status)
o2info_set_request_error(&oiff->iff_req, req); o2info_set_request_error(&oiff->iff_req, req);
out_free:
kfree(oiff); kfree(oiff);
out_err: out_err:
return status; return status;
...@@ -727,23 +690,17 @@ static int ocfs2_info_handle_freefrag(struct inode *inode, ...@@ -727,23 +690,17 @@ static int ocfs2_info_handle_freefrag(struct inode *inode,
static int ocfs2_info_handle_unknown(struct inode *inode, static int ocfs2_info_handle_unknown(struct inode *inode,
struct ocfs2_info_request __user *req) struct ocfs2_info_request __user *req)
{ {
int status = -EFAULT;
struct ocfs2_info_request oir; struct ocfs2_info_request oir;
if (o2info_from_user(oir, req)) if (o2info_from_user(oir, req))
goto bail; return -EFAULT;
o2info_clear_request_filled(&oir); o2info_clear_request_filled(&oir);
if (o2info_to_user(oir, req)) if (o2info_to_user(oir, req))
goto bail; return -EFAULT;
status = 0; return 0;
bail:
if (status)
o2info_set_request_error(&oir, req);
return status;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment