RE: [PATCH 1/1][MTD] Adding IOCTLs for run-time partitioning support

[Artem Bityutskiy <dedekind1@gmail.com>]

On Mon, 2010-09-06 at 08:04 +0530, Rohit Hassan Sathyanarayan wrote:
> +	case MTDPARTITIONCREATE:
> +	{
> +		u_int64_t	uoffset = 0;
> +		u_int64_t	usize = 0;
> +		u_int32_t	i;
> +
> +			if (copy_from_user(&upart, argp,
> +				sizeof(struct user_mtd_info))) {
> +				ret = -EFAULT;
> +				break;
> +			}
> +			ppart = kzalloc(sizeof(struct user_mtd)*
> +						upart.num_partitions
> +						, GFP_KERNEL);
> +			pmtd = kzalloc(sizeof(struct mtd_partition)*
> +					upart.num_partitions,
> +					GFP_KERNEL);
> +			pmtd1 = pmtd;
> +			if (ppart == NULL) {
> +				ret = -EFAULT;
> +				break;
> +			}
> +			if (copy_from_user(ppart,
> +					upart.pst_user_partitions,
> +					sizeof(struct user_mtd)*
> +					upart.num_partitions)) {
> +				ret = -EFAULT;
> +				break;
> +			}
> +			for (i = 0; i < upart.num_partitions; i++) {
> +				pmtd->name  = ppart->name;
> +				pmtd->size  =
> +					ppart->partition_size;
> +				usize += pmtd->size;
> +				pmtd->offset =
> +					ppart->partition_offset;
> +				uoffset += pmtd->offset;
> +				pmtd->mask_flags =
> +					ppart->partition_mask;
> +
> +				ppart++;
> +				pmtd++;
> +			}
> +			add_mtd_partitions(mtd, pmtd1,
> +						upart.num_partitions);
> +			if ((mtd->size - usize) > 0) {
> +				free.name	= "FREE";
> +				free.size	= mtd->size - usize;
> +				free.offset	= usize;
> +				free.mask_flags = 0;
> +				add_mtd_partitions(mtd, &free, 1);
> +			}
> +			kfree(pmtd1);
> +		break;
> +	}

So what happens if you already have many partitions, and this ioctl is
called? Where these existing partitions and the existing master device
will go? Who will free the resources occupied by them and where?

> +struct user_mtd_info {
> +	struct user_mtd *pst_user_partitions;
> +	__u32 num_partitions;
> +};

I suggested you to look at the discussion of Roman's patches - we did
discuss there that pointers in ioctls are bad, because then you have to
have implement compat_ioctl. You need to use u64 to avoid problems in
64-bit kernel + 32-bit user-space environments.


-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)