| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4f0177a1-0425-1de8-8e6f-836682d225b9@sancloud.com>
Date: Mon, 6 Dec 2021 10:42:20 +0000
From: Paul Barker <paul.barker@...cloud.com>
To: shiva.linuxworks@...il.com, tudor.ambarus@...rochip.com,
michael@...le.cc, p.yadav@...com, miquel.raynal@...tlin.com,
richard@....at, vigneshr@...com
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
Shivamurthy Shastri <sshivamurthy@...ron.com>
Subject: Re: [PATCH 3/4] mtd: add advanced protection and security ioctls
On 27/10/2021 11:33, shiva.linuxworks@...il.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@...ron.com>
>
> Added new ioctls for advanced protection and security features.
> These features are currently supported by new Micron SPI NOR flashes.
>
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@...ron.com>
> ---
> drivers/mtd/mtdchar.c | 145 +++++++++++++++++++++++++++++++++++++
> include/uapi/mtd/mtd-abi.h | 11 +++
> 2 files changed, 156 insertions(+)
>
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 155e991d9d75..97b97b80276d 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -654,6 +654,16 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
> case MTDFILEMODE:
> case BLKPG:
> case BLKRRPART:
> + case SECURE_PACKET_READ:
> + case SECURE_PACKET_WRITE:
> + case RD_VLOCK_BITS:
> + case WR_VLOCK_BITS:
> + case RD_NVLOCK_BITS:
> + case WR_NVLOCK_BITS:
> + case ER_NVLOCK_BITS:
> + case RD_GLOBAL_FREEZE_BITS:
> + case WR_GLOBAL_FREEZE_BITS:
> + case RD_PASSWORD:
> break;
It looks like you've listed all of the ioctls as "safe" commands so the
write permission bit is not checked. My understanding is that all ioctls
which may modify the data in flash need moving to the "dangerous"
commands section below this so that the write permission bit is checked.
>
> /* "dangerous" commands */
> @@ -1017,6 +1027,141 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
> ret = 0;
> break;
> }
> + case SECURE_PACKET_READ:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = kmalloc(buf.length, GFP_KERNEL);
> + ret = master->_secure_packet_read(master, buf.length, oobbuf);
> + if (copy_to_user(buf.ptr, oobbuf, buf.length))
> + ret = -EFAULT;
> + break;
> + }
> +
> + case SECURE_PACKET_WRITE:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = memdup_user(buf.ptr, buf.length);
> + ret = master->_secure_packet_write(master, buf.length, oobbuf);
> + break;
> + }
> +
> + case RD_VLOCK_BITS:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = kmalloc(buf.length, GFP_KERNEL);
> + ret = master->_read_vlock_bits(master, buf.start, buf.length,
> + oobbuf);
> + if (copy_to_user(buf.ptr, oobbuf, buf.length))
> + ret = -EFAULT;
> + break;
> + }
> +
> + case WR_VLOCK_BITS:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = memdup_user(buf.ptr, buf.length);
> + ret = master->_write_vlock_bits(master, buf. start, buf.length,
> + oobbuf);
> + break;
> + }
> +
> + case RD_NVLOCK_BITS:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = kmalloc(buf.length, GFP_KERNEL);
> + ret = master->_read_nvlock_bits(master, buf.start, buf.length,
> + oobbuf);
> + if (copy_to_user(buf.ptr, oobbuf, buf.length))
> + ret = -EFAULT;
> + break;
> + }
> +
> + case WR_NVLOCK_BITS:
> + {
> + struct mtd_oob_buf buf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + ret = master->_write_nvlock_bits(master, buf.start);
> + break;
> + }
> +
> + case ER_NVLOCK_BITS:
> + {
> + ret = master->_erase_nvlock_bits(master);
> + break;
> + }
> +
> + case RD_GLOBAL_FREEZE_BITS:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = kmalloc(buf.length, GFP_KERNEL);
> + ret = master->_read_global_freeze_bits(master, buf.length,
> + oobbuf);
> + if (copy_to_user(buf.ptr, oobbuf, buf.length))
> + ret = -EFAULT;
> + break;
> + }
> +
> + case WR_GLOBAL_FREEZE_BITS:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = memdup_user(buf.ptr, buf.length);
> + ret = master->_write_global_freeze_bits(master, buf.length,
> + oobbuf);
> + break;
> + }
> +
> + case RD_PASSWORD:
> + {
> + struct mtd_oob_buf buf;
> + u8 *oobbuf;
> +
> + if (copy_from_user(&buf, argp, sizeof(buf)))
> + ret = -EFAULT;
> +
> + oobbuf = kmalloc(buf.length, GFP_KERNEL);
> + ret = master->_read_password(master, buf.length, oobbuf);
> + if (copy_to_user(buf.ptr, oobbuf, buf.length))
> + ret = -EFAULT;
> + break;
> + }
> }
>
> return ret;
> diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
> index b869990c2db2..dbd7bf60d484 100644
> --- a/include/uapi/mtd/mtd-abi.h
> +++ b/include/uapi/mtd/mtd-abi.h
> @@ -208,6 +208,17 @@ struct otp_info {
> /* Erase a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
> #define OTPERASE _IOW('M', 25, struct otp_info)
>
> +#define SECURE_PACKET_READ _IOWR('M', 26, struct mtd_oob_buf)
> +#define SECURE_PACKET_WRITE _IOWR('M', 27, struct mtd_oob_buf)
> +#define RD_VLOCK_BITS _IOWR('M', 28, struct mtd_oob_buf)
> +#define WR_VLOCK_BITS _IOWR('M', 29, struct mtd_oob_buf)
> +#define RD_NVLOCK_BITS _IOWR('M', 30, struct mtd_oob_buf)
> +#define WR_NVLOCK_BITS _IOWR('M', 31, struct mtd_oob_buf)
> +#define ER_NVLOCK_BITS _IO('M', 32)
> +#define RD_GLOBAL_FREEZE_BITS _IOWR('M', 33, struct mtd_oob_buf)
> +#define WR_GLOBAL_FREEZE_BITS _IOWR('M', 34, struct mtd_oob_buf)
> +#define RD_PASSWORD _IOWR('M', 35, struct mtd_oob_buf)
> +
All other ioctls defined in this header are preceeded by a comment which
briefly explains what they do. I think this is needed for these new
ioctls as well.
> /*
> * Obsolete legacy interface. Keep it in order not to break userspace
> * interfaces
>
Thanks,
--
Paul Barker
Principal Software Engineer
SanCloud Ltd
e: paul.barker@...cloud.com
w: https://sancloud.co.uk/
Powered by blists - more mailing lists