[PATCH 1/1][MTD] Adding IOCTLs for run-time partitioning support
Roman Tereshonkov
roman.tereshonkov at nokia.com
Thu Sep 2 09:24:16 EDT 2010
On Thu, Jul 29, 2010 at 05:00:59PM +0530, Rohit Hassan Sathyanarayan wrote:
> Hi All
>
> Sending the patch on MTD for adding four IOCTLs.
> MTDPARTITIONCREATE
> MTDPARTITIONDELETE
> MTDPARTITIONSETPERMISSION
> MTDPARTITIONMERGE
>
>
>
> Signed-off-by: Rohit HS <rohit.hs at samsung.com>
> ---
> drivers/mtd/mtdchar.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/mtd/mtd-abi.h | 17 ++++++
> 2 files changed, 142 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 8b223c0..704788c 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -22,6 +22,10 @@
>
> #include <asm/uaccess.h>
>
> +#ifdef CONFIG_MTD_PARTITIONS
> +#include <linux/mtd/partitions.h>
> +#endif
> +
> #define MTD_INODE_FS_MAGIC 0x11307854
> static struct vfsmount *mtd_inode_mnt __read_mostly;
>
> @@ -826,6 +830,128 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
> }
> file->f_pos = 0;
> break;
> +#ifdef CONFIG_MTD_PARTITIONS
> +
> + case MTDPARTITIONCREATE:
> + {
> + struct mtd_partition *pst_minfo = NULL;
> + struct mtd_partition *pst_minfo_hold = NULL;
> + struct mtd_partition st_free;
> + uint64_t u64_userfreeoffset = 0;
> + uint64_t u64_userfreesize = 0;
> + struct user_mtd *puser_partitions = NULL;
> + struct user_mtd_info upartition;
> + int i;
> +
> + if (copy_from_user(&upartition, argp,
> + sizeof(struct user_mtd_info))) {
> + ret = -EFAULT;
> + break;
> + }
> + puser_partitions = kzalloc(sizeof(struct user_mtd)*
> + upartition.num_partitions
> + , GFP_KERNEL);
> + pst_minfo = kzalloc(sizeof(struct mtd_partition)*
> + upartition.num_partitions,
> + GFP_KERNEL);
> + pst_minfo_hold = pst_minfo;
> + if (puser_partitions == NULL) {
> + ret = -EFAULT;
> + break;
> + }
The case puser_partitions == NULL && pst_minfo != NULL leads to the memory leakage.
> + if (copy_from_user(puser_partitions,
> + upartition.pst_user_partitions,
> + sizeof(struct user_mtd)*
> + upartition.num_partitions)) {
> + ret = -EFAULT;
> + break;
What happens with previously allocated memory?
> + }
> + for (i = 0; i < upartition.num_partitions; i++) {
> + pst_minfo->name = puser_partitions->name;
> + pst_minfo->size =
> + puser_partitions->partition_size;
> + u64_userfreesize += pst_minfo->size;
> + pst_minfo->offset =
> + puser_partitions->partition_offset;
> + u64_userfreeoffset += pst_minfo->offset;
> + pst_minfo->mask_flags =
> + puser_partitions->partition_mask;
> +
> + puser_partitions++;
> + pst_minfo++;
> + }
> + add_mtd_partitions(mtd, pst_minfo_hold,
> + upartition.num_partitions);
The function add_mtd_partitions implies to be called only for all mtd partitions at once.
There are some dependences on the partition which goes the first in the passed list.
See add_one_partition function partno argument.
> + if ((mtd->size - u64_userfreesize) > 0) {
> + st_free.name = "FREE";
> + st_free.size = mtd->size - u64_userfreesize;
> + st_free.offset = u64_userfreesize;
> + st_free.mask_flags = 0;
> + add_mtd_partitions(mtd, &st_free, 1);
> + }
> + break;
> + }
> + case MTDPARTITIONDELETE:
> + {
How do you free memory allocated in MTDPARTITIONCREATE?
> + struct mtd_info *pst_del_info = NULL;
> + uint32_t mtd_partition_number;
> +
> + if (copy_from_user(&mtd_partition_number, argp,
> + sizeof(mtd_partition_number))) {
> + ret = -EFAULT;
> + break;
> + }
> + pst_del_info = get_mtd_device(NULL,
> + mtd_partition_number);
> +
> + if (pst_del_info != NULL) {
> + put_mtd_device(pst_del_info);
> + del_mtd_device(pst_del_info);
> + }
> + break;
> + }
If the partition is used del_mtd_device returns EBUSY.
Should the user space be informed about this?
> + case MTDPARTITIONSETPERMISSION:
> + {
> + uint32_t u32_mask_flags;
> + struct mtd_info *pst_device = NULL;
> + int i32_dev_num;
> +
> + if (copy_from_user(&u32_mask_flags, argp,
> + sizeof(u32_mask_flags))) {
> + ret = -EFAULT;
> + break;
> + }
> + i32_dev_num = (u32_mask_flags & 0x0000FFFF);
> + u32_mask_flags = (u32_mask_flags & 0xFFFF0000) >> 16;
> + pst_device = get_mtd_device(NULL, i32_dev_num);
> + pst_device->flags = u32_mask_flags;
> + break;
> + }
> + case MTDPARTITIONMERGE:
MERGE = DELETE + CREATE
Is this ioctl really needed?
> + {
> + struct mtd_info *pst_merge_device1 = NULL;
> + struct mtd_info *pst_merge_device2 = NULL;
> + unsigned char partition_number[2];
> +
> + if (copy_from_user(partition_number, argp, 2)) {
> + ret = -EFAULT;
> + break;
> + }
> + pst_merge_device1 = get_mtd_device(NULL,
> + partition_number[0]);
> + pst_merge_device2 = get_mtd_device(NULL,
> + partition_number[1]);
The case when only one of pst_merge_device1 and pst_merge_device2 is non-NULL
leads to the unreleased mtd_device.
> + if ((pst_merge_device1 != NULL) &&
> + (pst_merge_device2 != NULL)) {
> + pst_merge_device1->size +=
> + pst_merge_device2->size;
> + put_mtd_device(pst_merge_device2);
> + del_mtd_device(pst_merge_device2);
pst_merge_device1 should be released probably.
> + }
> + break;
> + }
> +#endif
> +
> }
>
> default:
> diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
> index be51ae2..85168f2 100644
> --- a/include/mtd/mtd-abi.h
> +++ b/include/mtd/mtd-abi.h
> @@ -30,6 +30,18 @@ struct mtd_oob_buf64 {
> __u64 usr_ptr;
> };
>
> +struct user_mtd {
> + char name[32];
> + __u64 partition_size;
> + __u64 partition_offset;
> + __u32 partition_mask;
> +};
> +
> +struct user_mtd_info {
> + struct user_mtd *pst_user_partitions;
> + __u32 num_partitions;
> +};
> +
> #define MTD_ABSENT 0
> #define MTD_RAM 1
> #define MTD_ROM 2
> @@ -110,7 +122,10 @@ struct otp_info {
> #define MEMERASE64 _IOW('M', 20, struct erase_info_user64)
> #define MEMWRITEOOB64 _IOWR('M', 21, struct mtd_oob_buf64)
> #define MEMREADOOB64 _IOWR('M', 22, struct mtd_oob_buf64)
> -
> +#define MTDPARTITIONCREATE _IOW('M', 23, int)
> +#define MTDPARTITIONDELETE _IOW('M', 24, int)
> +#define MTDPARTITIONSETPERMISSION _IOW('M', 26, int)
> +#define MTDPARTITIONMERGE _IOW('M', 27, int)
> /*
> * Obsolete legacy interface. Keep it in order not to break userspace
> * interfaces
> ---
>
>
More information about the linux-mtd
mailing list