[PATCH] [MTD] CORE: New ioctl calls for >4GiB device support
Adrian Hunter
adrian.hunter at nokia.com
Wed Mar 18 06:26:07 EDT 2009
Kevin Cernekee wrote:
> Extend the MTD user ABI to access >4GiB devices using 64-bit offsets.
>
> New ioctls: MEMGETINFO64 MEMERASE64 MEMWRITEOOB64 MEMREADOOB64 MEMLOCK64
> MEMUNLOCK64 MEMGETREGIONINFO64
>
> Signed-off-by: Kevin Cernekee <kpc.mtd at gmail.com>
> ---
What about compat ioctls?
> drivers/mtd/mtdchar.c | 128 ++++++++++++++++++++++++++++++++++++++++++-----
> include/mtd/mtd-abi.h | 36 +++++++++++++
> include/mtd/mtd-user.h | 2 +
> 3 files changed, 152 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index e9ec59e..4bde913 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -387,6 +387,7 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
> int ret = 0;
> u_long size;
> struct mtd_info_user info;
> + struct mtd_info_user64 info64;
>
> DEBUG(MTD_DEBUG_LEVEL0, "MTD_ioctl\n");
>
> @@ -425,6 +426,26 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> break;
> }
>
> + case MEMGETREGIONINFO64:
> + {
> + uint32_t ur_idx;
> + struct mtd_erase_region_info *kr;
> + struct region_info_user64 *ur =
> + (struct region_info_user64 *) argp;
> +
> + if (get_user(ur_idx, &(ur->regionindex)))
> + return -EFAULT;
> +
> + kr = &(mtd->eraseregions[ur_idx]);
> +
> + if (put_user(kr->offset, &(ur->offset))
> + || put_user(kr->erasesize, &(ur->erasesize))
> + || put_user(kr->numblocks, &(ur->numblocks)))
> + return -EFAULT;
> +
> + break;
> + }
> +
> case MEMGETINFO:
> info.type = mtd->type;
> info.flags = mtd->flags;
> @@ -439,7 +460,19 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> return -EFAULT;
> break;
>
> + case MEMGETINFO64:
> + info64.type = mtd->type;
> + info64.flags = mtd->flags;
> + info64.size = mtd->size;
> + info64.erasesize = mtd->erasesize;
> + info64.writesize = mtd->writesize;
> + info64.oobsize = mtd->oobsize;
> + if (copy_to_user(argp, &info64, sizeof(struct mtd_info_user64)))
> + return -EFAULT;
> + break;
> +
> case MEMERASE:
> + case MEMERASE64:
> {
> struct erase_info *erase;
>
> @@ -450,20 +483,32 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> if (!erase)
> ret = -ENOMEM;
> else {
> - struct erase_info_user einfo;
> -
> wait_queue_head_t waitq;
> DECLARE_WAITQUEUE(wait, current);
>
> init_waitqueue_head(&waitq);
>
> - if (copy_from_user(&einfo, argp,
> - sizeof(struct erase_info_user))) {
> - kfree(erase);
> - return -EFAULT;
> + if(cmd == MEMERASE64) {
> + struct erase_info_user64 einfo64;
> +
> + if (copy_from_user(&einfo64, argp,
> + sizeof(struct erase_info_user64))) {
> + kfree(erase);
> + return -EFAULT;
> + }
> + erase->addr = einfo64.start;
> + erase->len = einfo64.length;
> + } else {
> + struct erase_info_user einfo32;
> +
> + if (copy_from_user(&einfo32, argp,
> + sizeof(struct erase_info_user))) {
> + kfree(erase);
> + return -EFAULT;
> + }
> + erase->addr = einfo32.start;
> + erase->len = einfo32.length;
> }
> - erase->addr = einfo.start;
> - erase->len = einfo.length;
> erase->mtd = mtd;
> erase->callback = mtdchar_erase_callback;
> erase->priv = (unsigned long)&waitq;
> @@ -495,8 +540,9 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
> }
>
> case MEMWRITEOOB:
> + case MEMWRITEOOB64:
> {
> - struct mtd_oob_buf buf;
> + struct mtd_oob_buf64 buf;
> struct mtd_oob_ops ops;
> struct mtd_oob_buf __user *user_buf = argp;
> uint32_t retlen;
> @@ -504,8 +550,21 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> if(!(file->f_mode & FMODE_WRITE))
> return -EPERM;
>
> - if (copy_from_user(&buf, argp, sizeof(struct mtd_oob_buf)))
> - return -EFAULT;
> + if (cmd == MEMWRITEOOB64) {
> + if (copy_from_user(&buf, argp,
> + sizeof(struct mtd_oob_buf64)))
> + return -EFAULT;
> + } else {
> + struct mtd_oob_buf buf32;
> +
> + if (copy_from_user(&buf32, argp,
> + sizeof(struct mtd_oob_buf)))
> + return -EFAULT;
> +
> + buf.start = buf32.start;
> + buf.length = buf32.length;
> + buf.ptr = buf32.ptr;
> + }
>
> if (buf.length > 4096)
> return -EINVAL;
This bit is not 64-bit safe:
buf.start &= ~(mtd->oobsize - 1);
This bit needs to use the right structure:
if (copy_to_user(&user_buf->length, &retlen, sizeof(buf.length)))
ret = -EFAULT;
> @@ -551,12 +610,25 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> }
>
> case MEMREADOOB:
> + case MEMREADOOB64:
> {
> - struct mtd_oob_buf buf;
> + struct mtd_oob_buf64 buf;
> struct mtd_oob_ops ops;
>
> - if (copy_from_user(&buf, argp, sizeof(struct mtd_oob_buf)))
> - return -EFAULT;
> + if (cmd == MEMREADOOB64) {
> + if (copy_from_user(&buf, argp,
> + sizeof(struct mtd_oob_buf64)))
> + return -EFAULT;
> + } else {
> + struct mtd_oob_buf buf32;
> +
> + if (copy_from_user(&buf32, argp,
> + sizeof(struct mtd_oob_buf)))
> + return -EFAULT;
> + buf.start = buf32.start;
> + buf.length = buf32.length;
> + buf.ptr = buf32.ptr;
> + }
>
> if (buf.length > 4096)
> return -EINVAL;
Same as above:
buf.start &= ~(mtd->oobsize - 1);
This bit needs attention:
if (put_user(ops.oobretlen, (uint32_t __user *)argp))
ret = -EFAULT;
argp points to 'start' not 'length' and 'start' may be 64 or 32 bits.
That original behaviour is kind-of a bug but it has consequences for
userspace, so it is not clear what the best course of action is.
Refer:
http://lists.infradead.org/pipermail/linux-mtd/2009-January/024194.html
> @@ -608,6 +680,20 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> break;
> }
>
> + case MEMLOCK64:
> + {
> + struct erase_info_user64 einfo64;
> +
> + if (copy_from_user(&einfo64, argp, sizeof(einfo64)))
> + return -EFAULT;
> +
> + if (!mtd->lock)
> + ret = -EOPNOTSUPP;
> + else
> + ret = mtd->lock(mtd, einfo64.start, einfo64.length);
> + break;
> + }
> +
> case MEMUNLOCK:
> {
> struct erase_info_user einfo;
> @@ -622,6 +708,20 @@ static int mtd_ioctl(struct inode *inode, struct
> file *file,
> break;
> }
>
> + case MEMUNLOCK64:
> + {
> + struct erase_info_user64 einfo64;
> +
> + if (copy_from_user(&einfo64, argp, sizeof(einfo64)))
> + return -EFAULT;
> +
> + if (!mtd->unlock)
> + ret = -EOPNOTSUPP;
> + else
> + ret = mtd->unlock(mtd, einfo64.start, einfo64.length);
> + break;
> + }
> +
> /* Legacy interface */
> case MEMGETOOBSEL:
> {
> diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
> index c6c61cd..10ebb7c 100644
> --- a/include/mtd/mtd-abi.h
> +++ b/include/mtd/mtd-abi.h
> @@ -10,12 +10,23 @@ struct erase_info_user {
> uint32_t length;
> };
>
> +struct erase_info_user64 {
> + uint64_t start;
> + uint64_t length;
> +};
> +
> struct mtd_oob_buf {
> uint32_t start;
> uint32_t length;
> unsigned char __user *ptr;
> };
>
> +struct mtd_oob_buf64 {
> + uint64_t start;
> + uint32_t length;
> + unsigned char __user *ptr;
> +};
> +
> #define MTD_ABSENT 0
> #define MTD_RAM 1
> #define MTD_ROM 2
> @@ -60,6 +71,15 @@ struct mtd_info_user {
> uint32_t eccsize;
> };
>
> +struct mtd_info_user64 {
> + uint32_t type;
> + uint32_t flags;
> + uint64_t size; // Total size of the MTD
> + uint32_t erasesize;
> + uint32_t writesize;
> + uint32_t oobsize; // Amount of OOB data per block (e.g. 16)
> +};
> +
> struct region_info_user {
> uint32_t offset; /* At which this region starts,
> * from the beginning of the MTD */
> @@ -68,6 +88,14 @@ struct region_info_user {
> uint32_t regionindex;
> };
>
> +struct region_info_user64 {
> + uint64_t offset; /* At which this region starts,
> + * from the beginning of the MTD */
> + uint32_t erasesize; /* For this region */
> + uint32_t numblocks; /* Number of blocks in this region */
> + uint32_t regionindex;
> +};
> +
> struct otp_info {
> uint32_t start;
> uint32_t length;
> @@ -94,6 +122,14 @@ struct otp_info {
> #define ECCGETSTATS _IOR('M', 18, struct mtd_ecc_stats)
> #define MTDFILEMODE _IO('M', 19)
>
> +#define MEMGETINFO64 _IOR('M', 20, struct mtd_info_user64)
> +#define MEMERASE64 _IOW('M', 21, struct erase_info_user64)
> +#define MEMWRITEOOB64 _IOWR('M', 22, struct mtd_oob_buf64)
> +#define MEMREADOOB64 _IOWR('M', 23, struct mtd_oob_buf64)
> +#define MEMLOCK64 _IOW('M', 24, struct erase_info_user64)
> +#define MEMUNLOCK64 _IOW('M', 25, struct erase_info_user64)
> +#define MEMGETREGIONINFO64 _IOWR('M', 26, struct region_info_user64)
> +
> /*
> * Obsolete legacy interface. Keep it in order not to break userspace
> * interfaces
> diff --git a/include/mtd/mtd-user.h b/include/mtd/mtd-user.h
> index 170ceca..30e55a2 100644
> --- a/include/mtd/mtd-user.h
> +++ b/include/mtd/mtd-user.h
> @@ -7,6 +7,8 @@
>
> #include <stdint.h>
>
> +#define __user
> +
Don't do that. Use "make headers_install" to make headers for user space.
> /* This file is blessed for inclusion by userspace */
> #include <mtd/mtd-abi.h>
>
More information about the linux-mtd
mailing list