diff mbox series

[-next,1/3] md: refactor md ioctl cmd check

Message ID 20220920023938.3273598-2-yebin10@huawei.com (mailing list archive)
State Rejected, archived
Delegated to: Song Liu
Headers show
Series do some cleanup for md_ioctl | expand

Commit Message

yebin (H) Sept. 20, 2022, 2:39 a.m. UTC 
Rename md_ioctl_valid to  md_ioctl_check, check whether the command is valid
and whether the permission passes.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 drivers/md/md.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

Comments

Christoph Hellwig Sept. 20, 2022, 7:19 a.m. UTC | #1 
On Tue, Sep 20, 2022 at 10:39:36AM +0800, Ye Bin wrote:
> Rename md_ioctl_valid to  md_ioctl_check, check whether the command is valid
> and whether the permission passes.

I think this going into entirely the wrong direction.  The right way
to go is to split each handler into a separate function, and move
those checks there.  I have a WIP series for that which I just need to
dust off.
diff mbox series

Patch

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 729be2c5296c..19960bb33f6d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7440,16 +7440,17 @@  static int md_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	return 0;
 }
 
-static inline bool md_ioctl_valid(unsigned int cmd)
+static inline int md_ioctl_check(unsigned int cmd)
 {
 	switch (cmd) {
-	case ADD_NEW_DISK:
+	case RAID_VERSION:
 	case GET_ARRAY_INFO:
-	case GET_BITMAP_FILE:
 	case GET_DISK_INFO:
+		break;
+	case ADD_NEW_DISK:
+	case GET_BITMAP_FILE:
 	case HOT_ADD_DISK:
 	case HOT_REMOVE_DISK:
-	case RAID_VERSION:
 	case RESTART_ARRAY_RW:
 	case RUN_ARRAY:
 	case SET_ARRAY_INFO:
@@ -7458,10 +7459,14 @@  static inline bool md_ioctl_valid(unsigned int cmd)
 	case STOP_ARRAY:
 	case STOP_ARRAY_RO:
 	case CLUSTERED_DISK_NACK:
-		return true;
+		if (!capable(CAP_SYS_ADMIN))
+			return -EACCES;
+		break;
 	default:
-		return false;
+		return -ENOTTY;
 	}
+
+	return 0;
 }
 
 static int md_ioctl(struct block_device *bdev, fmode_t mode,
@@ -7472,18 +7477,9 @@  static int md_ioctl(struct block_device *bdev, fmode_t mode,
 	struct mddev *mddev = NULL;
 	bool did_set_md_closing = false;
 
-	if (!md_ioctl_valid(cmd))
-		return -ENOTTY;
-
-	switch (cmd) {
-	case RAID_VERSION:
-	case GET_ARRAY_INFO:
-	case GET_DISK_INFO:
-		break;
-	default:
-		if (!capable(CAP_SYS_ADMIN))
-			return -EACCES;
-	}
+	err = md_ioctl_check(cmd);
+	if (err)
+		return err;
 
 	/*
 	 * Commands dealing with the RAID driver but not any