| Message ID | alpine.DEB.2.00.1104051450480.25803@peruna (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2011/4/5 John Calixto <john.calixto@modsystems.com>: > Sending ACMDs from userspace is useful for such things as: > > - The security application of an SD card (SD Specification, Part 3, > Security) > > - SD passthrough for virtual machines > > Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 > SoC, TI OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC. > > Signed-off-by: John Calixto <john.calixto@modsystems.com> > --- > drivers/mmc/card/block.c | 191 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/mmc/core/sd_ops.c | 3 +- > include/linux/mmc/core.h | 1 + > include/linux/mmc/sd.h | 16 ++++ > 4 files changed, 210 insertions(+), 1 deletions(-) > > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c > index 61d233a..c2e107c 100644 > --- a/drivers/mmc/card/block.c > +++ b/drivers/mmc/card/block.c > @@ -31,6 +31,8 @@ > #include <linux/mutex.h> > #include <linux/scatterlist.h> > #include <linux/string_helpers.h> > +#include <linux/compat.h> > +#include <linux/delay.h> > > #include <linux/mmc/card.h> > #include <linux/mmc/host.h> > @@ -158,11 +160,200 @@ mmc_blk_getgeo(struct block_device *bdev, struct hd_geometry *geo) > return 0; > } > > +static int mmc_blk_ioctl_acmd(struct block_device *bdev, > + struct sd_ioc_cmd __user *sdic_ptr) > +{ > + struct sd_ioc_cmd sdic; > + struct mmc_blk_data *md; > + struct mmc_host *host; > + struct mmc_card *card; > + struct mmc_command cmd = {0}; > + struct mmc_data data = {0}; > + struct mmc_request mrq = {0}; > + struct scatterlist sg = {0}; > + unsigned char *blocks = NULL; > + size_t data_bytes; > + int err; > + > + /* > + * Only allow ACMDs on the whole block device, not on partitions. This > + * prevents overspray between sibling partitions. > + */ > + if (bdev != bdev->bd_contains) > + return -EPERM; This should at least also check CAP_SYS_ADMIN capability. > + > + md = mmc_blk_get(bdev->bd_disk); > + if (!md) > + return -EINVAL; > + > + card = md->queue.card; > + if (IS_ERR(card)) > + return PTR_ERR(card); > + > + host = card->host; > + mmc_claim_host(host); > + > + err = mmc_app_cmd(host, card); > + if (err) > + goto acmd_done; > + > + mrq.cmd = &cmd; > + mrq.data = &data; > + > + if (copy_from_user(&sdic, sdic_ptr, sizeof(sdic))) { > + err = -EFAULT; > + goto acmd_done; > + } You should first copy and verify ioctl's data and only then claim host and send commands. Preferably the check-and-copy should be separate function. > + > + cmd.opcode = sdic.opcode; > + cmd.arg = sdic.arg; > + cmd.flags = sdic.flags; > + > + data.sg = &sg; > + data.sg_len = 1; > + data.blksz = sdic.blksz; > + data.blocks = sdic.blocks; > + > + data_bytes = data.blksz * data.blocks; > + blocks = kzalloc(data_bytes, GFP_KERNEL); > + if (!blocks) { > + err = -ENOMEM; > + goto acmd_done; > + } > + sg_init_one(data.sg, blocks, data_bytes); > + > + > + if (copy_from_user(blocks, sdic_ptr->data_ptr, data_bytes)) { > + err = -EFAULT; > + goto acmd_done; > + } > + if (sdic.write_flag) > + data.flags = MMC_DATA_WRITE; > + else > + data.flags = MMC_DATA_READ; > + > + /* data.flags must already be set before doing this. */ > + mmc_set_data_timeout(&data, card); > + /* Allow overriding the timeout_ns for empirical tuning. */ > + if (sdic.force_timeout_ns) > + data.timeout_ns = sdic.force_timeout_ns; > + > + mmc_wait_for_req(host, &mrq); > + > + if (cmd.error) { > + dev_err(mmc_dev(host), "%s: cmd error %d\n", > + __func__, cmd.error); > + err = cmd.error; > + goto acmd_done; > + } > + if (data.error) { > + dev_err(mmc_dev(host), "%s: data error %d\n", > + __func__, data.error); > + err = data.error; > + goto acmd_done; > + } > + > + /* > + * According to the SD specs, some commands require a delay after > + * issuing the command. > + */ > + if (sdic.postsleep_us) > + udelay(sdic.postsleep_us); > + > + if (copy_to_user(&(sdic_ptr->response), cmd.resp, sizeof(cmd.resp))) { > + err = -EFAULT; > + goto acmd_done; > + } > + > + if (!sdic.write_flag) { > + if (copy_to_user(sdic_ptr->data_ptr, blocks, data_bytes)) { > + err = -EFAULT; > + goto acmd_done; > + } > + } > + > +acmd_done: > + kfree(blocks); > + mmc_release_host(host); > + mmc_blk_put(md); > + return err; > +} > + > +static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode, > + unsigned int cmd, unsigned long arg) > +{ > + int ret = -EINVAL; > + mutex_lock(&block_mutex); > + if (cmd == SD_IOC_ACMD) > + ret = mmc_blk_ioctl_acmd(bdev, (struct sd_ioc_cmd __user *)arg); > + mutex_unlock(&block_mutex); > + return ret; > +} > + > +#ifdef CONFIG_COMPAT > +struct sd_ioc_cmd32 { > + u32 write_flag; > + u32 opcode; > + u32 arg; > + u32 response[4]; > + u32 flags; > + u32 blksz; > + u32 blocks; > + u32 postsleep_us; > + u32 force_timeout_ns; > + compat_uptr_t data_ptr; > +}; > +#define SD_IOC_ACMD32 _IOWR(MMC_BLOCK_MAJOR, 0, struct sd_ioc_cmd32) [...] Since your implementing a new ioctl you can make the structure the same for 64 and 32-bit archs and avoid all this compat crap. Best Regards, Micha? Miros?aw -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 6 Apr 2011, Micha? Miros?aw wrote: > 2011/4/5 John Calixto <john.calixto@modsystems.com>: > > + /* > > + * Only allow ACMDs on the whole block device, not on partitions. This > > + * prevents overspray between sibling partitions. > > + */ > > + if (bdev != bdev->bd_contains) > > + return -EPERM; > > This should at least also check CAP_SYS_ADMIN capability. > Yep. I broke out the capability check into a separate patch - I'll reply to your permissions-related comments on that thread. > > + > > + md = mmc_blk_get(bdev->bd_disk); > > + if (!md) > > + return -EINVAL; > > + > > + card = md->queue.card; > > + if (IS_ERR(card)) > > + return PTR_ERR(card); > > + > > + host = card->host; > > + mmc_claim_host(host); > > + > > + err = mmc_app_cmd(host, card); > > + if (err) > > + goto acmd_done; > > + > > + mrq.cmd = &cmd; > > + mrq.data = &data; > > + > > + if (copy_from_user(&sdic, sdic_ptr, sizeof(sdic))) { > > + err = -EFAULT; > > + goto acmd_done; > > + } > > You should first copy and verify ioctl's data and only then claim host > and send commands. Preferably the check-and-copy should be separate > function. > Will do. > > + > > +#ifdef CONFIG_COMPAT > > +struct sd_ioc_cmd32 { > > + u32 write_flag; > > + u32 opcode; > > + u32 arg; > > + u32 response[4]; > > + u32 flags; > > + u32 blksz; > > + u32 blocks; > > + u32 postsleep_us; > > + u32 force_timeout_ns; > > + compat_uptr_t data_ptr; > > +}; > > +#define SD_IOC_ACMD32 _IOWR(MMC_BLOCK_MAJOR, 0, struct sd_ioc_cmd32) > [...] > > Since your implementing a new ioctl you can make the structure the > same for 64 and 32-bit archs and avoid all this compat crap. > I was also less than pleased with this, but I chose to implement the compat crap because it allow a "natural" userspace API for both the kernel32+user32 and kernel64+user64 environments. The ugliness in the kernel is just when you have defined CONFIG_COMPAT. I think 32-bit-only is still an important target for this functionality (think set-top media players, mobile devices; basically, anything running on ARM) so always having to cast your data pointer to 64-bit is not appealing. I suspect it will be very unlikely to see people using this in the kernel64+user32 environment. Cheers, John
W dniu 6 kwietnia 2011 19:37 u?ytkownik John Calixto <john.calixto@modsystems.com> napisa?: > On Wed, 6 Apr 2011, Micha? Miros?aw wrote: >> 2011/4/5 John Calixto <john.calixto@modsystems.com>: >> > +#ifdef CONFIG_COMPAT >> > +struct sd_ioc_cmd32 { >> > + u32 write_flag; >> > + u32 opcode; >> > + u32 arg; >> > + u32 response[4]; >> > + u32 flags; >> > + u32 blksz; >> > + u32 blocks; >> > + u32 postsleep_us; >> > + u32 force_timeout_ns; >> > + compat_uptr_t data_ptr; >> > +}; >> > +#define SD_IOC_ACMD32 _IOWR(MMC_BLOCK_MAJOR, 0, struct sd_ioc_cmd32) >> [...] >> >> Since your implementing a new ioctl you can make the structure the >> same for 64 and 32-bit archs and avoid all this compat crap. > I was also less than pleased with this, but I chose to implement the > compat crap because it allow a "natural" userspace API for both the > kernel32+user32 and kernel64+user64 environments. The ugliness in the > kernel is just when you have defined CONFIG_COMPAT. I think 32-bit-only > is still an important target for this functionality (think set-top media > players, mobile devices; basically, anything running on ARM) so always > having to cast your data pointer to 64-bit is not appealing. I suspect > it will be very unlikely to see people using this in the kernel64+user32 > environment. The problem is only with data_ptr field. If you make it the same size whether it's for 32-bit or 64-bit arch then you don't need all that compat code for user32 on kernel64 except for pointer translation when there's 32-bit process calling. Best Regards, Micha? Miros?aw -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 61d233a..c2e107c 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -31,6 +31,8 @@ #include <linux/mutex.h> #include <linux/scatterlist.h> #include <linux/string_helpers.h> +#include <linux/compat.h> +#include <linux/delay.h> #include <linux/mmc/card.h> #include <linux/mmc/host.h> @@ -158,11 +160,200 @@ mmc_blk_getgeo(struct block_device *bdev, struct hd_geometry *geo) return 0; } +static int mmc_blk_ioctl_acmd(struct block_device *bdev, + struct sd_ioc_cmd __user *sdic_ptr) +{ + struct sd_ioc_cmd sdic; + struct mmc_blk_data *md; + struct mmc_host *host; + struct mmc_card *card; + struct mmc_command cmd = {0}; + struct mmc_data data = {0}; + struct mmc_request mrq = {0}; + struct scatterlist sg = {0}; + unsigned char *blocks = NULL; + size_t data_bytes; + int err; + + /* + * Only allow ACMDs on the whole block device, not on partitions. This + * prevents overspray between sibling partitions. + */ + if (bdev != bdev->bd_contains) + return -EPERM; + + md = mmc_blk_get(bdev->bd_disk); + if (!md) + return -EINVAL; + + card = md->queue.card; + if (IS_ERR(card)) + return PTR_ERR(card); + + host = card->host; + mmc_claim_host(host); + + err = mmc_app_cmd(host, card); + if (err) + goto acmd_done; + + mrq.cmd = &cmd; + mrq.data = &data; + + if (copy_from_user(&sdic, sdic_ptr, sizeof(sdic))) { + err = -EFAULT; + goto acmd_done; + } + + cmd.opcode = sdic.opcode; + cmd.arg = sdic.arg; + cmd.flags = sdic.flags; + + data.sg = &sg; + data.sg_len = 1; + data.blksz = sdic.blksz; + data.blocks = sdic.blocks; + + data_bytes = data.blksz * data.blocks; + blocks = kzalloc(data_bytes, GFP_KERNEL); + if (!blocks) { + err = -ENOMEM; + goto acmd_done; + } + sg_init_one(data.sg, blocks, data_bytes); + + + if (copy_from_user(blocks, sdic_ptr->data_ptr, data_bytes)) { + err = -EFAULT; + goto acmd_done; + } + if (sdic.write_flag) + data.flags = MMC_DATA_WRITE; + else + data.flags = MMC_DATA_READ; + + /* data.flags must already be set before doing this. */ + mmc_set_data_timeout(&data, card); + /* Allow overriding the timeout_ns for empirical tuning. */ + if (sdic.force_timeout_ns) + data.timeout_ns = sdic.force_timeout_ns; + + mmc_wait_for_req(host, &mrq); + + if (cmd.error) { + dev_err(mmc_dev(host), "%s: cmd error %d\n", + __func__, cmd.error); + err = cmd.error; + goto acmd_done; + } + if (data.error) { + dev_err(mmc_dev(host), "%s: data error %d\n", + __func__, data.error); + err = data.error; + goto acmd_done; + } + + /* + * According to the SD specs, some commands require a delay after + * issuing the command. + */ + if (sdic.postsleep_us) + udelay(sdic.postsleep_us); + + if (copy_to_user(&(sdic_ptr->response), cmd.resp, sizeof(cmd.resp))) { + err = -EFAULT; + goto acmd_done; + } + + if (!sdic.write_flag) { + if (copy_to_user(sdic_ptr->data_ptr, blocks, data_bytes)) { + err = -EFAULT; + goto acmd_done; + } + } + +acmd_done: + kfree(blocks); + mmc_release_host(host); + mmc_blk_put(md); + return err; +} + +static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode, + unsigned int cmd, unsigned long arg) +{ + int ret = -EINVAL; + mutex_lock(&block_mutex); + if (cmd == SD_IOC_ACMD) + ret = mmc_blk_ioctl_acmd(bdev, (struct sd_ioc_cmd __user *)arg); + mutex_unlock(&block_mutex); + return ret; +} + +#ifdef CONFIG_COMPAT +struct sd_ioc_cmd32 { + u32 write_flag; + u32 opcode; + u32 arg; + u32 response[4]; + u32 flags; + u32 blksz; + u32 blocks; + u32 postsleep_us; + u32 force_timeout_ns; + compat_uptr_t data_ptr; +}; +#define SD_IOC_ACMD32 _IOWR(MMC_BLOCK_MAJOR, 0, struct sd_ioc_cmd32) + +static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode, + unsigned int cmd, unsigned long arg) +{ + struct sd_ioc_cmd32 __user *sdic32 = (struct sd_ioc_cmd32 __user *) arg; + struct sd_ioc_cmd sdic; + struct sd_ioc_cmd __user *tmp = compat_alloc_user_space(sizeof(sdic)); + u32 cp; + int err; + + if (cmd != SD_IOC_ACMD32) + return -ENOIOCTLCMD; + + err = 0; + err |= get_user(sdic.write_flag, &sdic32->write_flag); + err |= get_user(sdic.opcode, &sdic32->opcode); + err |= get_user(sdic.arg, &sdic32->arg); + err |= get_user(sdic.flags, &sdic32->flags); + err |= get_user(sdic.blksz, &sdic32->blksz); + err |= get_user(sdic.blocks, &sdic32->blocks); + err |= get_user(sdic.postsleep_us, &sdic32->postsleep_us); + err |= get_user(sdic.force_timeout_ns, &sdic32->force_timeout_ns); + err |= get_user(cp, &sdic32->data_ptr); + sdic.data_ptr = compat_ptr(cp); + err |= copy_to_user(tmp, &sdic, sizeof(sdic)); + if (err) + return -EFAULT; + + err = mmc_blk_ioctl(bdev, mode, SD_IOC_ACMD, (unsigned long) tmp); + if (err) + return err; + + err |= copy_in_user(&sdic32->response, &tmp->response, + sizeof(sdic32->response)); + if (err) + return -EFAULT; + + return err; +} +#endif + static const struct block_device_operations mmc_bdops = { .open = mmc_blk_open, .release = mmc_blk_release, .getgeo = mmc_blk_getgeo, .owner = THIS_MODULE, + .ioctl = mmc_blk_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = mmc_blk_compat_ioctl, +#endif }; struct mmc_blk_request { diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index 797cdb5..990dd43 100644 --- a/drivers/mmc/core/sd_ops.c +++ b/drivers/mmc/core/sd_ops.c @@ -20,7 +20,7 @@ #include "core.h" #include "sd_ops.h" -static int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) +int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) { int err; struct mmc_command cmd; @@ -48,6 +48,7 @@ static int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) return 0; } +EXPORT_SYMBOL_GPL(mmc_app_cmd); /** * mmc_wait_for_app_cmd - start an application command and wait for diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index 07f27af..bfc6127 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -133,6 +133,7 @@ struct mmc_card; extern void mmc_wait_for_req(struct mmc_host *, struct mmc_request *); extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int); +extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *); extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, struct mmc_command *, int); diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index 3fd85e0..8ead3c8 100644 --- a/include/linux/mmc/sd.h +++ b/include/linux/mmc/sd.h @@ -12,6 +12,8 @@ #ifndef MMC_SD_H #define MMC_SD_H +#include <linux/ioctl.h> + /* SD commands type argument response */ /* class 0 */ /* This is basically the same command as for MMC with some quirks. */ @@ -84,5 +86,19 @@ #define SD_SWITCH_ACCESS_DEF 0 #define SD_SWITCH_ACCESS_HS 1 +struct sd_ioc_cmd { + int write_flag; /* implies direction of data. true = write, false = read */ + __u32 opcode; + __u32 arg; + __u32 response[4]; /* CMD response */ + unsigned int flags; + unsigned int blksz; + unsigned int blocks; + unsigned int postsleep_us; /* apply usecond delay *after* issuing command */ + unsigned int force_timeout_ns; /* force timeout to be force_timeout_ns ns */ + __u8 *data_ptr; /* DAT buffer */ +}; +#define SD_IOC_ACMD _IOWR(MMC_BLOCK_MAJOR, 0, struct sd_ioc_cmd) + #endif
Sending ACMDs from userspace is useful for such things as: - The security application of an SD card (SD Specification, Part 3, Security) - SD passthrough for virtual machines Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 SoC, TI OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC. Signed-off-by: John Calixto <john.calixto@modsystems.com> --- drivers/mmc/card/block.c | 191 +++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/sd_ops.c | 3 +- include/linux/mmc/core.h | 1 + include/linux/mmc/sd.h | 16 ++++ 4 files changed, 210 insertions(+), 1 deletions(-)