[PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[David Kozub]

This patch series extends SED Opal support: it adds IOCTL for setting the shadow
MBR done flag which can be useful for unlocking an Opal disk on boot and it adds
IOCTL for writing to the shadow MBR.

The code has been already submitted in [1] but it was suggested to split this
part off. In comparison to that version, I tried to apply Scott's suggestions
from [2].

This series requires the previously-submitted Opal cleanup patches[3]. It
applies on current linux-next/master (e.g. next-20190430).

I successfully tested toggling the MBR done flag and writing the shadow MBR
using some tools I hacked together[4] with a Samsung SSD 850 EVO drive.

[1] https://lore.kernel.org/lkml/1549054223-12220-1-git-send-email-zub@linux.fjfi.cvut.cz/
[2] https://lore.kernel.org/lkml/20190210182655.GA20491@hacktheplanet/
[3] https://lore.kernel.org/lkml/1550103368-4605-1-git-send-email-zub@linux.fjfi.cvut.cz/
[4] https://gitlab.com/zub2/opalctl

Jonas Rabenstein (3):
  block: sed-opal: add ioctl for done-mark of shadow mbr
  block: sed-opal: ioctl for writing to shadow mbr
  block: sed-opal: check size of shadow mbr

 block/opal_proto.h            |  16 ++++
 block/sed-opal.c              | 160 +++++++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |   2 +
 include/uapi/linux/sed-opal.h |  20 +++++
 4 files changed, 196 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH 1/3] block: sed-opal: add ioctl for done-mark of shadow mbr

[David Kozub]

From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>

Enable users to mark the shadow mbr as done without completely
deactivating the shadow mbr feature. This may be useful on reboots,
when the power to the disk is not disconnected in between and the shadow
mbr stores the required boot files. Of course, this saves also the
(few) commands required to enable the feature if it is already enabled
and one only wants to mark the shadow mbr as done.

Co-authored-by: David Kozub <zub@linux.fjfi.cvut.cz>
Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
---
 block/sed-opal.c              | 27 +++++++++++++++++++++++++++
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h | 12 ++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index b1aa0cc25803..f1eb9c18e335 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -1986,6 +1986,30 @@ static int opal_enable_disable_shadow_mbr(struct opal_dev *dev,
 	return ret;
 }
 
+static int opal_set_mbr_done(struct opal_dev *dev,
+			     struct opal_mbr_done *mbr_done)
+{
+	u8 mbr_done_tf = mbr_done->done_flag == OPAL_MBR_DONE ?
+		OPAL_TRUE : OPAL_FALSE;
+
+	const struct opal_step mbr_steps[] = {
+		{ start_admin1LSP_opal_session, &mbr_done->key },
+		{ set_mbr_done, &mbr_done_tf },
+		{ end_opal_session, }
+	};
+	int ret;
+
+	if (mbr_done->done_flag != OPAL_MBR_DONE &&
+	    mbr_done->done_flag != OPAL_MBR_NOT_DONE)
+		return -EINVAL;
+
+	mutex_lock(&dev->dev_lock);
+	setup_opal_dev(dev);
+	ret = execute_steps(dev, mbr_steps, ARRAY_SIZE(mbr_steps));
+	mutex_unlock(&dev->dev_lock);
+	return ret;
+}
+
 static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
 {
 	struct opal_suspend_data *suspend;
@@ -2299,6 +2323,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 	case IOC_OPAL_ENABLE_DISABLE_MBR:
 		ret = opal_enable_disable_shadow_mbr(dev, p);
 		break;
+	case IOC_OPAL_MBR_DONE:
+		ret = opal_set_mbr_done(dev, p);
+		break;
 	case IOC_OPAL_ERASE_LR:
 		ret = opal_erase_locking_range(dev, p);
 		break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 04b124fca51e..42b2ce5da7b3 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	case IOC_OPAL_ENABLE_DISABLE_MBR:
 	case IOC_OPAL_ERASE_LR:
 	case IOC_OPAL_SECURE_ERASE_LR:
+	case IOC_OPAL_MBR_DONE:
 		return true;
 	}
 	return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index e092e124dd16..81dd0e8886a1 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -29,6 +29,11 @@ enum opal_mbr {
 	OPAL_MBR_DISABLE = 0x01,
 };
 
+enum opal_mbr_done_flag {
+	OPAL_MBR_NOT_DONE = 0x0,
+	OPAL_MBR_DONE = 0x01
+};
+
 enum opal_user {
 	OPAL_ADMIN1 = 0x0,
 	OPAL_USER1 = 0x01,
@@ -104,6 +109,12 @@ struct opal_mbr_data {
 	__u8 __align[7];
 };
 
+struct opal_mbr_done {
+	struct opal_key key;
+	__u8 done_flag;
+	__u8 __align[7];
+};
+
 #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
@@ -116,5 +127,6 @@ struct opal_mbr_data {
 #define IOC_OPAL_ENABLE_DISABLE_MBR _IOW('p', 229, struct opal_mbr_data)
 #define IOC_OPAL_ERASE_LR           _IOW('p', 230, struct opal_session_info)
 #define IOC_OPAL_SECURE_ERASE_LR    _IOW('p', 231, struct opal_session_info)
+#define IOC_OPAL_MBR_DONE           _IOW('p', 232, struct opal_mbr_done)
 
 #endif /* _UAPI_SED_OPAL_H */
-- 
2.20.1

Re: [PATCH 1/3] block: sed-opal: add ioctl for done-mark of shadow mbr

[David Kozub]

On Wed, 1 May 2019, David Kozub wrote:

> @@ -104,6 +109,12 @@ struct opal_mbr_data {
> 	__u8 __align[7];
> };
>
> +struct opal_mbr_done {
> +	struct opal_key key;
> +	__u8 done_flag;
> +	__u8 __align[7];
> +};

While I just copied opal_mbr_data here, I wonder what is the point of 
__align in these structs. By itself it just pads the structure to have a 
size that is a multiple of 8. Is this to make sure that anything that lies 
past the structure is 8-bytes aligned (assuming the start is 8-bytes 
aligned too), perhaps for 32bit userspace with 64bit kernel?

And if it's this, is it needed for these IOCTL structs? (I can see it 
being useful for struct opal_key.)

Best regards,
David

Re: [PATCH 1/3] block: sed-opal: add ioctl for done-mark of shadow mbr

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/3] block: sed-opal: add ioctl for done-mark of shadow mbr

[Scott Bauer]

On Wed, May 01, 2019 at 01:20:57AM +0200, David Kozub wrote:
> From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> 
> Enable users to mark the shadow mbr as done without completely
> deactivating the shadow mbr feature. This may be useful on reboots,
> when the power to the disk is not disconnected in between and the shadow
> mbr stores the required boot files. Of course, this saves also the
> (few) commands required to enable the feature if it is already enabled
> and one only wants to mark the shadow mbr as done.
> 
> Co-authored-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
Looks fine.
Reviewed by: Scott Bauer <sbauer@plzdonthack.me>

Re: [PATCH 1/3] block: sed-opal: add ioctl for done-mark of shadow mbr

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 4204 bytes --]

LGTM

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>

On Wed, 2019-05-01 at 01:20 +0200, David Kozub wrote:
> From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> 
> Enable users to mark the shadow mbr as done without completely
> deactivating the shadow mbr feature. This may be useful on reboots,
> when the power to the disk is not disconnected in between and the
> shadow
> mbr stores the required boot files. Of course, this saves also the
> (few) commands required to enable the feature if it is already
> enabled
> and one only wants to mark the shadow mbr as done.
> 
> Co-authored-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Signed-off-by: Jonas Rabenstein <
> jonas.rabenstein@studium.uni-erlangen.de>
> Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
> ---
>  block/sed-opal.c              | 27 +++++++++++++++++++++++++++
>  include/linux/sed-opal.h      |  1 +
>  include/uapi/linux/sed-opal.h | 12 ++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index b1aa0cc25803..f1eb9c18e335 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -1986,6 +1986,30 @@ static int
> opal_enable_disable_shadow_mbr(struct opal_dev *dev,
>  	return ret;
>  }
>  
> +static int opal_set_mbr_done(struct opal_dev *dev,
> +			     struct opal_mbr_done *mbr_done)
> +{
> +	u8 mbr_done_tf = mbr_done->done_flag == OPAL_MBR_DONE ?
> +		OPAL_TRUE : OPAL_FALSE;
> +
> +	const struct opal_step mbr_steps[] = {
> +		{ start_admin1LSP_opal_session, &mbr_done->key },
> +		{ set_mbr_done, &mbr_done_tf },
> +		{ end_opal_session, }
> +	};
> +	int ret;
> +
> +	if (mbr_done->done_flag != OPAL_MBR_DONE &&
> +	    mbr_done->done_flag != OPAL_MBR_NOT_DONE)
> +		return -EINVAL;
> +
> +	mutex_lock(&dev->dev_lock);
> +	setup_opal_dev(dev);
> +	ret = execute_steps(dev, mbr_steps, ARRAY_SIZE(mbr_steps));
> +	mutex_unlock(&dev->dev_lock);
> +	return ret;
> +}
> +
>  static int opal_save(struct opal_dev *dev, struct opal_lock_unlock
> *lk_unlk)
>  {
>  	struct opal_suspend_data *suspend;
> @@ -2299,6 +2323,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned
> int cmd, void __user *arg)
>  	case IOC_OPAL_ENABLE_DISABLE_MBR:
>  		ret = opal_enable_disable_shadow_mbr(dev, p);
>  		break;
> +	case IOC_OPAL_MBR_DONE:
> +		ret = opal_set_mbr_done(dev, p);
> +		break;
>  	case IOC_OPAL_ERASE_LR:
>  		ret = opal_erase_locking_range(dev, p);
>  		break;
> diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
> index 04b124fca51e..42b2ce5da7b3 100644
> --- a/include/linux/sed-opal.h
> +++ b/include/linux/sed-opal.h
> @@ -47,6 +47,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
>  	case IOC_OPAL_ENABLE_DISABLE_MBR:
>  	case IOC_OPAL_ERASE_LR:
>  	case IOC_OPAL_SECURE_ERASE_LR:
> +	case IOC_OPAL_MBR_DONE:
>  		return true;
>  	}
>  	return false;
> diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-
> opal.h
> index e092e124dd16..81dd0e8886a1 100644
> --- a/include/uapi/linux/sed-opal.h
> +++ b/include/uapi/linux/sed-opal.h
> @@ -29,6 +29,11 @@ enum opal_mbr {
>  	OPAL_MBR_DISABLE = 0x01,
>  };
>  
> +enum opal_mbr_done_flag {
> +	OPAL_MBR_NOT_DONE = 0x0,
> +	OPAL_MBR_DONE = 0x01
> +};
> +
>  enum opal_user {
>  	OPAL_ADMIN1 = 0x0,
>  	OPAL_USER1 = 0x01,
> @@ -104,6 +109,12 @@ struct opal_mbr_data {
>  	__u8 __align[7];
>  };
>  
> +struct opal_mbr_done {
> +	struct opal_key key;
> +	__u8 done_flag;
> +	__u8 __align[7];
> +};
> +
>  #define IOC_OPAL_SAVE		    _IOW('p', 220, struct
> opal_lock_unlock)
>  #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct
> opal_lock_unlock)
>  #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct
> opal_key)
> @@ -116,5 +127,6 @@ struct opal_mbr_data {
>  #define IOC_OPAL_ENABLE_DISABLE_MBR _IOW('p', 229, struct
> opal_mbr_data)
>  #define IOC_OPAL_ERASE_LR           _IOW('p', 230, struct
> opal_session_info)
>  #define IOC_OPAL_SECURE_ERASE_LR    _IOW('p', 231, struct
> opal_session_info)
> +#define IOC_OPAL_MBR_DONE           _IOW('p', 232, struct
> opal_mbr_done)
>  
>  #endif /* _UAPI_SED_OPAL_H */

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

[PATCH 2/3] block: sed-opal: ioctl for writing to shadow mbr

[David Kozub]

From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>

Allow modification of the shadow mbr. If the shadow mbr is not marked as
done, this data will be presented read only as the device content. Only
after marking the shadow mbr as done and unlocking a locking range the
actual content is accessible.

Co-authored-by: David Kozub <zub@linux.fjfi.cvut.cz>
Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
Reviewed-by: Scott Bauer <sbauer@plzdonthack.me>
Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c              | 94 ++++++++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h |  8 +++
 3 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index f1eb9c18e335..5acb873e9037 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -34,6 +34,9 @@
 #define IO_BUFFER_LENGTH 2048
 #define MAX_TOKS 64
 
+/* Number of bytes needed by cmd_finalize. */
+#define CMD_FINALIZE_BYTES_NEEDED 7
+
 struct opal_step {
 	int (*fn)(struct opal_dev *dev, void *data);
 	void *data;
@@ -531,12 +534,17 @@ static int opal_discovery0_step(struct opal_dev *dev)
 	return execute_step(dev, &discovery0_step, 0);
 }
 
+static size_t remaining_size(struct opal_dev *cmd)
+{
+	return IO_BUFFER_LENGTH - cmd->pos;
+}
+
 static bool can_add(int *err, struct opal_dev *cmd, size_t len)
 {
 	if (*err)
 		return false;
 
-	if (len > IO_BUFFER_LENGTH || cmd->pos > IO_BUFFER_LENGTH - len) {
+	if (remaining_size(cmd) < len) {
 		pr_debug("Error adding %zu bytes: end of buffer.\n", len);
 		*err = -ERANGE;
 		return false;
@@ -682,7 +690,11 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
 	struct opal_header *hdr;
 	int err = 0;
 
-	/* close the parameter list opened from cmd_start */
+	/*
+	 * Close the parameter list opened from cmd_start.
+	 * The number of bytes added must be equal to
+	 * CMD_FINALIZE_BYTES_NEEDED.
+	 */
 	add_token_u8(&err, cmd, OPAL_ENDLIST);
 
 	add_token_u8(&err, cmd, OPAL_ENDOFDATA);
@@ -1533,6 +1545,58 @@ static int set_mbr_enable_disable(struct opal_dev *dev, void *data)
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
+static int write_shadow_mbr(struct opal_dev *dev, void *data)
+{
+	struct opal_shadow_mbr *shadow = data;
+	const u8 __user *src;
+	u8 *dst;
+	size_t off = 0;
+	u64 len;
+	int err = 0;
+
+	/* do the actual transmission(s) */
+	src = (u8 __user *)(uintptr_t)shadow->data;
+	while (off < shadow->size) {
+		err = cmd_start(dev, opaluid[OPAL_MBR], opalmethod[OPAL_SET]);
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_WHERE);
+		add_token_u64(&err, dev, shadow->offset + off);
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+
+		add_token_u8(&err, dev, OPAL_STARTNAME);
+		add_token_u8(&err, dev, OPAL_VALUES);
+
+		/*
+		 * The bytestring header is either 1 or 2 bytes, so assume 2.
+		 * There also needs to be enough space to accommodate the
+		 * trailing OPAL_ENDNAME (1 byte) and tokens added by
+		 * cmd_finalize.
+		 */
+		len = min(remaining_size(dev) - (2+1+CMD_FINALIZE_BYTES_NEEDED),
+			  (size_t)(shadow->size - off));
+		pr_debug("MBR: write bytes %zu+%llu/%llu\n",
+			 off, len, shadow->size);
+
+		dst = add_bytestring_header(&err, dev, len);
+		if (!dst)
+			break;
+		if (copy_from_user(dst, src + off, len))
+			err = -EFAULT;
+		dev->pos += len;
+
+		add_token_u8(&err, dev, OPAL_ENDNAME);
+		if (err)
+			break;
+
+		err = finalize_and_send(dev, parse_and_check_status);
+		if (err)
+			break;
+
+		off += len;
+	}
+	return err;
+}
+
 static int generic_pw_cmd(u8 *key, size_t key_len, u8 *cpin_uid,
 			  struct opal_dev *dev)
 {
@@ -2010,6 +2074,29 @@ static int opal_set_mbr_done(struct opal_dev *dev,
 	return ret;
 }
 
+static int opal_write_shadow_mbr(struct opal_dev *dev,
+				 struct opal_shadow_mbr *info)
+{
+	const struct opal_step mbr_steps[] = {
+		{ start_admin1LSP_opal_session, &info->key },
+		{ write_shadow_mbr, info },
+		{ end_opal_session, }
+	};
+	int ret;
+
+	if (info->size == 0)
+		return 0;
+
+	if (!access_ok((void __user *)(uintptr_t)info->data, info->size))
+		return -EFAULT;
+
+	mutex_lock(&dev->dev_lock);
+	setup_opal_dev(dev);
+	ret = execute_steps(dev, mbr_steps, ARRAY_SIZE(mbr_steps));
+	mutex_unlock(&dev->dev_lock);
+	return ret;
+}
+
 static int opal_save(struct opal_dev *dev, struct opal_lock_unlock *lk_unlk)
 {
 	struct opal_suspend_data *suspend;
@@ -2326,6 +2413,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 	case IOC_OPAL_MBR_DONE:
 		ret = opal_set_mbr_done(dev, p);
 		break;
+	case IOC_OPAL_WRITE_SHADOW_MBR:
+		ret = opal_write_shadow_mbr(dev, p);
+		break;
 	case IOC_OPAL_ERASE_LR:
 		ret = opal_erase_locking_range(dev, p);
 		break;
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 42b2ce5da7b3..fb5bf6138a98 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -48,6 +48,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	case IOC_OPAL_ERASE_LR:
 	case IOC_OPAL_SECURE_ERASE_LR:
 	case IOC_OPAL_MBR_DONE:
+	case IOC_OPAL_WRITE_SHADOW_MBR:
 		return true;
 	}
 	return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 81dd0e8886a1..9c83785f0a2a 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -115,6 +115,13 @@ struct opal_mbr_done {
 	__u8 __align[7];
 };
 
+struct opal_shadow_mbr {
+	struct opal_key key;
+	const __u64 data;
+	__u64 offset;
+	__u64 size;
+};
+
 #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
@@ -128,5 +135,6 @@ struct opal_mbr_done {
 #define IOC_OPAL_ERASE_LR           _IOW('p', 230, struct opal_session_info)
 #define IOC_OPAL_SECURE_ERASE_LR    _IOW('p', 231, struct opal_session_info)
 #define IOC_OPAL_MBR_DONE           _IOW('p', 232, struct opal_mbr_done)
+#define IOC_OPAL_WRITE_SHADOW_MBR   _IOW('p', 233, struct opal_shadow_mbr)
 
 #endif /* _UAPI_SED_OPAL_H */
-- 
2.20.1

Re: [PATCH 2/3] block: sed-opal: ioctl for writing to shadow mbr

[Christoph Hellwig]

> +	if (!access_ok((void __user *)(uintptr_t)info->data, info->size))
> +		return -EFAULT;

Just let the copy_from_user itself return the error itself.

Re: [PATCH 2/3] block: sed-opal: ioctl for writing to shadow mbr

[Scott Bauer]

On Wed, May 01, 2019 at 01:20:58AM +0200, David Kozub wrote:
> From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> 
> Allow modification of the shadow mbr. If the shadow mbr is not marked as
> done, this data will be presented read only as the device content. Only
> after marking the shadow mbr as done and unlocking a locking range the
> actual content is accessible.
> 
> Co-authored-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Reviewed-by: Scott Bauer <sbauer@plzdonthack.me>
> Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>


re-reviewed and looks fine: Scott Bauer <sbauer@plzdonthack.me>

[PATCH 3/3] block: sed-opal: check size of shadow mbr

[David Kozub]

From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>

Check whether the shadow mbr does fit in the provided space on the
target. Also a proper firmware should handle this case and return an
error we may prevent problems or even damage with crappy firmwares.

Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
Reviewed-by: Scott Bauer <sbauer@plzdonthack.me>
Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/opal_proto.h | 16 ++++++++++++++++
 block/sed-opal.c   | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/block/opal_proto.h b/block/opal_proto.h
index b6e352cfe982..5e8df3245eb0 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -106,6 +106,7 @@ enum opal_uid {
 	OPAL_ENTERPRISE_BANDMASTER0_UID,
 	OPAL_ENTERPRISE_ERASEMASTER_UID,
 	/* tables */
+	OPAL_TABLE_TABLE,
 	OPAL_LOCKINGRANGE_GLOBAL,
 	OPAL_LOCKINGRANGE_ACE_RDLOCKED,
 	OPAL_LOCKINGRANGE_ACE_WRLOCKED,
@@ -160,6 +161,21 @@ enum opal_token {
 	OPAL_STARTCOLUMN = 0x03,
 	OPAL_ENDCOLUMN = 0x04,
 	OPAL_VALUES = 0x01,
+	/* table table */
+	OPAL_TABLE_UID = 0x00,
+	OPAL_TABLE_NAME = 0x01,
+	OPAL_TABLE_COMMON = 0x02,
+	OPAL_TABLE_TEMPLATE = 0x03,
+	OPAL_TABLE_KIND = 0x04,
+	OPAL_TABLE_COLUMN = 0x05,
+	OPAL_TABLE_COLUMNS = 0x06,
+	OPAL_TABLE_ROWS = 0x07,
+	OPAL_TABLE_ROWS_FREE = 0x08,
+	OPAL_TABLE_ROW_BYTES = 0x09,
+	OPAL_TABLE_LASTID = 0x0A,
+	OPAL_TABLE_MIN = 0x0B,
+	OPAL_TABLE_MAX = 0x0C,
+
 	/* authority table */
 	OPAL_PIN = 0x03,
 	/* locking tokens */
diff --git a/block/sed-opal.c b/block/sed-opal.c
index 5acb873e9037..39e3eecca58d 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -138,6 +138,8 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
 
 	/* tables */
 
+	[OPAL_TABLE_TABLE]
+		{ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
 	[OPAL_LOCKINGRANGE_GLOBAL] =
 		{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
 	[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
@@ -1139,6 +1141,29 @@ static int generic_get_column(struct opal_dev *dev, const u8 *table,
 	return finalize_and_send(dev, parse_and_check_status);
 }
 
+/*
+ * see TCG SAS 5.3.2.3 for a description of the available columns
+ *
+ * the result is provided in dev->resp->tok[4]
+ */
+static int generic_get_table_info(struct opal_dev *dev, enum opal_uid table,
+				  u64 column)
+{
+	u8 uid[OPAL_UID_LENGTH];
+	const unsigned int half = OPAL_UID_LENGTH/2;
+
+	/* sed-opal UIDs can be split in two halves:
+	 *  first:  actual table index
+	 *  second: relative index in the table
+	 * so we have to get the first half of the OPAL_TABLE_TABLE and use the
+	 * first part of the target table as relative index into that table
+	 */
+	memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
+	memcpy(uid+half, opaluid[table], half);
+
+	return generic_get_column(dev, uid, column);
+}
+
 static int gen_key(struct opal_dev *dev, void *data)
 {
 	u8 uid[OPAL_UID_LENGTH];
@@ -1554,6 +1579,20 @@ static int write_shadow_mbr(struct opal_dev *dev, void *data)
 	u64 len;
 	int err = 0;
 
+	/* do we fit in the available shadow mbr space? */
+	err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
+	if (err) {
+		pr_debug("MBR: could not get shadow size\n");
+		return err;
+	}
+
+	len = response_get_u64(&dev->parsed, 4);
+	if (shadow->size > len || shadow->offset > len - shadow->size) {
+		pr_debug("MBR: does not fit in shadow (%llu vs. %llu)\n",
+			 shadow->offset + shadow->size, len);
+		return -ENOSPC;
+	}
+
 	/* do the actual transmission(s) */
 	src = (u8 __user *)(uintptr_t)shadow->data;
 	while (off < shadow->size) {
-- 
2.20.1

Re: [PATCH 3/3] block: sed-opal: check size of shadow mbr

[Scott Bauer]

On Wed, May 01, 2019 at 01:20:59AM +0200, David Kozub wrote:
> From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> 
> Check whether the shadow mbr does fit in the provided space on the
> target. Also a proper firmware should handle this case and return an
> error we may prevent problems or even damage with crappy firmwares.
> 
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Reviewed-by: Scott Bauer <sbauer@plzdonthack.me>
> Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
re-reviewed and looks fine again: Scott Bauer <sbauer@plzdonthack.me>

Re: [PATCH 3/3] block: sed-opal: check size of shadow mbr

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]

lgtm again

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>

On Wed, 2019-05-01 at 01:20 +0200, David Kozub wrote:
> From: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> 
> Check whether the shadow mbr does fit in the provided space on the
> target. Also a proper firmware should handle this case and return an
> error we may prevent problems or even damage with crappy firmwares.
> 
> Signed-off-by: Jonas Rabenstein <
> jonas.rabenstein@studium.uni-erlangen.de>
> Signed-off-by: David Kozub <zub@linux.fjfi.cvut.cz>
> Reviewed-by: Scott Bauer <sbauer@plzdonthack.me>
> Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  block/opal_proto.h | 16 ++++++++++++++++
>  block/sed-opal.c   | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/block/opal_proto.h b/block/opal_proto.h
> index b6e352cfe982..5e8df3245eb0 100644
> --- a/block/opal_proto.h
> +++ b/block/opal_proto.h
> @@ -106,6 +106,7 @@ enum opal_uid {
>  	OPAL_ENTERPRISE_BANDMASTER0_UID,
>  	OPAL_ENTERPRISE_ERASEMASTER_UID,
>  	/* tables */
> +	OPAL_TABLE_TABLE,
>  	OPAL_LOCKINGRANGE_GLOBAL,
>  	OPAL_LOCKINGRANGE_ACE_RDLOCKED,
>  	OPAL_LOCKINGRANGE_ACE_WRLOCKED,
> @@ -160,6 +161,21 @@ enum opal_token {
>  	OPAL_STARTCOLUMN = 0x03,
>  	OPAL_ENDCOLUMN = 0x04,
>  	OPAL_VALUES = 0x01,
> +	/* table table */
> +	OPAL_TABLE_UID = 0x00,
> +	OPAL_TABLE_NAME = 0x01,
> +	OPAL_TABLE_COMMON = 0x02,
> +	OPAL_TABLE_TEMPLATE = 0x03,
> +	OPAL_TABLE_KIND = 0x04,
> +	OPAL_TABLE_COLUMN = 0x05,
> +	OPAL_TABLE_COLUMNS = 0x06,
> +	OPAL_TABLE_ROWS = 0x07,
> +	OPAL_TABLE_ROWS_FREE = 0x08,
> +	OPAL_TABLE_ROW_BYTES = 0x09,
> +	OPAL_TABLE_LASTID = 0x0A,
> +	OPAL_TABLE_MIN = 0x0B,
> +	OPAL_TABLE_MAX = 0x0C,
> +
>  	/* authority table */
>  	OPAL_PIN = 0x03,
>  	/* locking tokens */
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 5acb873e9037..39e3eecca58d 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -138,6 +138,8 @@ static const u8 opaluid[][OPAL_UID_LENGTH] = {
>  
>  	/* tables */
>  
> +	[OPAL_TABLE_TABLE]
> +		{ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01 },
>  	[OPAL_LOCKINGRANGE_GLOBAL] =
>  		{ 0x00, 0x00, 0x08, 0x02, 0x00, 0x00, 0x00, 0x01 },
>  	[OPAL_LOCKINGRANGE_ACE_RDLOCKED] =
> @@ -1139,6 +1141,29 @@ static int generic_get_column(struct opal_dev
> *dev, const u8 *table,
>  	return finalize_and_send(dev, parse_and_check_status);
>  }
>  
> +/*
> + * see TCG SAS 5.3.2.3 for a description of the available columns
> + *
> + * the result is provided in dev->resp->tok[4]
> + */
> +static int generic_get_table_info(struct opal_dev *dev, enum
> opal_uid table,
> +				  u64 column)
> +{
> +	u8 uid[OPAL_UID_LENGTH];
> +	const unsigned int half = OPAL_UID_LENGTH/2;
> +
> +	/* sed-opal UIDs can be split in two halves:
> +	 *  first:  actual table index
> +	 *  second: relative index in the table
> +	 * so we have to get the first half of the OPAL_TABLE_TABLE and
> use the
> +	 * first part of the target table as relative index into that
> table
> +	 */
> +	memcpy(uid, opaluid[OPAL_TABLE_TABLE], half);
> +	memcpy(uid+half, opaluid[table], half);
> +
> +	return generic_get_column(dev, uid, column);
> +}
> +
>  static int gen_key(struct opal_dev *dev, void *data)
>  {
>  	u8 uid[OPAL_UID_LENGTH];
> @@ -1554,6 +1579,20 @@ static int write_shadow_mbr(struct opal_dev
> *dev, void *data)
>  	u64 len;
>  	int err = 0;
>  
> +	/* do we fit in the available shadow mbr space? */
> +	err = generic_get_table_info(dev, OPAL_MBR, OPAL_TABLE_ROWS);
> +	if (err) {
> +		pr_debug("MBR: could not get shadow size\n");
> +		return err;
> +	}
> +
> +	len = response_get_u64(&dev->parsed, 4);
> +	if (shadow->size > len || shadow->offset > len - shadow->size)
> {
> +		pr_debug("MBR: does not fit in shadow (%llu vs.
> %llu)\n",
> +			 shadow->offset + shadow->size, len);
> +		return -ENOSPC;
> +	}
> +
>  	/* do the actual transmission(s) */
>  	src = (u8 __user *)(uintptr_t)shadow->data;
>  	while (off < shadow->size) {

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[Christoph Hellwig]

> I successfully tested toggling the MBR done flag and writing the shadow MBR
> using some tools I hacked together[4] with a Samsung SSD 850 EVO drive.

Can you submit the tool to util-linux so that we get it into distros?

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[David Kozub]

On Wed, 1 May 2019, Christoph Hellwig wrote:

>> I successfully tested toggling the MBR done flag and writing the shadow MBR
>> using some tools I hacked together[4] with a Samsung SSD 850 EVO drive.
>
> Can you submit the tool to util-linux so that we get it into distros?

There is already Scott's sed-opal-temp[1] and a fork by Jonas that adds 
support for older version of these new IOCTLs[2]. There was already some 
discussion of getting that to util-linux.[3]

While I like my hack, sed-opal-temp can do much more (my tool supports 
just the few things I actually use). But there are two things which 
sed-opal-temp currently lacks which my hack has:

* It can use a PBKDF2 hash (salted by disk serial number) of the password
   rather than the password directly. This makes it compatible with sedutil
   and I think it's also better practice (as firmware can contain many
   surprises).

* It contains a 'PBA' (pre-boot authorization) tool. A tool intended to be
   run from shadow mbr that asks for a password and uses it to unlock all
   disks and set shadow mbr done flag, so after restart the computer boots
   into the real OS.

@Scott: What are your plans with sed-opal-temp? If you want I can update 
Jonas' patches to the adapted IOCTLs. What are your thoughts on PW hashing 
and a PBA tool?

Best regards,
David

[1] https://github.com/ScottyBauer/sed-opal-temp
[2] https://github.com/ghostav/sed-opal-temp
[3] https://lkml.org/lkml/2019/2/4/736

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[Scott Bauer]

On Fri, May 03, 2019 at 10:32:19PM +0200, David Kozub wrote:
> On Wed, 1 May 2019, Christoph Hellwig wrote:
> 
> > > I successfully tested toggling the MBR done flag and writing the shadow MBR
> > > using some tools I hacked together[4] with a Samsung SSD 850 EVO drive.
> > 
> > Can you submit the tool to util-linux so that we get it into distros?
> 
> There is already Scott's sed-opal-temp[1] and a fork by Jonas that adds
> support for older version of these new IOCTLs[2]. There was already some
> discussion of getting that to util-linux.[3]
> 
> While I like my hack, sed-opal-temp can do much more (my tool supports just
> the few things I actually use). But there are two things which sed-opal-temp
> currently lacks which my hack has:
> 
> * It can use a PBKDF2 hash (salted by disk serial number) of the password
>   rather than the password directly. This makes it compatible with sedutil
>   and I think it's also better practice (as firmware can contain many
>   surprises).
> 
> * It contains a 'PBA' (pre-boot authorization) tool. A tool intended to be
>   run from shadow mbr that asks for a password and uses it to unlock all
>   disks and set shadow mbr done flag, so after restart the computer boots
>   into the real OS.
> 
> @Scott: What are your plans with sed-opal-temp? If you want I can update
> Jonas' patches to the adapted IOCTLs. What are your thoughts on PW hashing
> and a PBA tool?

I will accept any and all patches to sed opal tooling, I am not picky. I will
also give up maintainership of it is someone else feels they can (rightfully
so) do a better job.

Jon sent me a patch for the tool that will deal with writing to the shadow MBR,
so once we know these patches are going in i'll pull that patch into the tool.

Then I guess that leaves PBKDF2 which I don't think will be too hard to pull in.

With regard to your PBA tool, is that actually being run post-uefi/pre-linux?
IE are we writing your tool into the SMBR and that's what is being run on bootup?

Jon, if you think it's a good idea can you ask David if Revanth or you wants
to take over the tooling? Or if anyone else here wants to own it then let me know.

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]

On Sun, 2019-05-05 at 10:43 -0400, Scott Bauer wrote:
> On Fri, May 03, 2019 at 10:32:19PM +0200, David Kozub wrote:
> > On Wed, 1 May 2019, Christoph Hellwig wrote:
> > 
> > > > I successfully tested toggling the MBR done flag and writing
> > > > the shadow MBR
> > > > using some tools I hacked together[4] with a Samsung SSD 850
> > > > EVO drive.
> > > 
> > > Can you submit the tool to util-linux so that we get it into
> > > distros?
> > 
> > There is already Scott's sed-opal-temp[1] and a fork by Jonas that
> > adds
> > support for older version of these new IOCTLs[2]. There was already
> > some
> > discussion of getting that to util-linux.[3]
> > 
> > While I like my hack, sed-opal-temp can do much more (my tool
> > supports just
> > the few things I actually use). But there are two things which sed-
> > opal-temp
> > currently lacks which my hack has:
> > 
> > * It can use a PBKDF2 hash (salted by disk serial number) of the
> > password
> >   rather than the password directly. This makes it compatible with
> > sedutil
> >   and I think it's also better practice (as firmware can contain
> > many
> >   surprises).
> > 
> > * It contains a 'PBA' (pre-boot authorization) tool. A tool
> > intended to be
> >   run from shadow mbr that asks for a password and uses it to
> > unlock all
> >   disks and set shadow mbr done flag, so after restart the computer
> > boots
> >   into the real OS.
> > 
> > @Scott: What are your plans with sed-opal-temp? If you want I can
> > update
> > Jonas' patches to the adapted IOCTLs. What are your thoughts on PW
> > hashing
> > and a PBA tool?
> 
> I will accept any and all patches to sed opal tooling, I am not
> picky. I will
> also give up maintainership of it is someone else feels they can
> (rightfully
> so) do a better job.
> 
> Jon sent me a patch for the tool that will deal with writing to the
> shadow MBR,
> so once we know these patches are going in i'll pull that patch into
> the tool.
> 
> Then I guess that leaves PBKDF2 which I don't think will be too hard
> to pull in.
> 
> With regard to your PBA tool, is that actually being run post-
> uefi/pre-linux?
> IE are we writing your tool into the SMBR and that's what is being
> run on bootup?
> 
> Jon, if you think it's a good idea can you ask David if Revanth or
> you wants
> to take over the tooling? Or if anyone else here wants to own it then
> let me know.
> 

I'll get back to you on this. Let me know if it begins to pick up a lot
of steam and I can prioritize this.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[David Kozub]

On Sun, 5 May 2019, Scott Bauer wrote:

> On Fri, May 03, 2019 at 10:32:19PM +0200, David Kozub wrote:
>> On Wed, 1 May 2019, Christoph Hellwig wrote:
>>
>>>> I successfully tested toggling the MBR done flag and writing the shadow MBR
>>>> using some tools I hacked together[4] with a Samsung SSD 850 EVO drive.
>>>
>>> Can you submit the tool to util-linux so that we get it into distros?
>>
>> There is already Scott's sed-opal-temp[1] and a fork by Jonas that adds
>> support for older version of these new IOCTLs[2]. There was already some
>> discussion of getting that to util-linux.[3]
>>
>> While I like my hack, sed-opal-temp can do much more (my tool supports just
>> the few things I actually use). But there are two things which sed-opal-temp
>> currently lacks which my hack has:
>>
>> * It can use a PBKDF2 hash (salted by disk serial number) of the password
>>   rather than the password directly. This makes it compatible with sedutil
>>   and I think it's also better practice (as firmware can contain many
>>   surprises).
>>
>> * It contains a 'PBA' (pre-boot authorization) tool. A tool intended to be
>>   run from shadow mbr that asks for a password and uses it to unlock all
>>   disks and set shadow mbr done flag, so after restart the computer boots
>>   into the real OS.
>>
>> @Scott: What are your plans with sed-opal-temp? If you want I can update
>> Jonas' patches to the adapted IOCTLs. What are your thoughts on PW hashing
>> and a PBA tool?
>
> I will accept any and all patches to sed opal tooling, I am not picky. I will
> also give up maintainership of it is someone else feels they can (rightfully
> so) do a better job.
>
> Jon sent me a patch for the tool that will deal with writing to the shadow MBR,
> so once we know these patches are going in i'll pull that patch into the tool.
>
> Then I guess that leaves PBKDF2 which I don't think will be too hard to pull in.

After (if) these patches are accepted, I can create a patch that adds it 
to sed-opal-temp.

> With regard to your PBA tool, is that actually being run post-uefi/pre-linux?
> IE are we writing your tool into the SMBR and that's what is being run on bootup?

It's actually nothing fancy: It's just a linux program that asks for a 
password and then uses it to unlock all block devices. It's intended to be 
run from an initramfs. So the idea is you build a kernel + initramfs with 
the tool so that the tool it started and after the tool returns, initramfs 
does a reboot. This could be replaced just by a shell script, though then 
you'd have to pass the password from the shell script to e.g. 
sed-opal-temp.

But I think it covers the simple scenario: booting from a locked drive 
with just one locking range. Possibly with other locked drives connected 
that also have one locking range and use the same password (when using pwd 
hash at least the Opal key is not the same).

> Jon, if you think it's a good idea can you ask David if Revanth or you wants
> to take over the tooling? Or if anyone else here wants to own it then let me know.

I got invoved in this just to scratch an itch so I would not be a good 
candidate.

Best regards,
David

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[Scott Bauer]

On Wed, May 01, 2019 at 01:20:56AM +0200, David Kozub wrote:
> 
> Jonas Rabenstein (3):
>   block: sed-opal: add ioctl for done-mark of shadow mbr
>   block: sed-opal: ioctl for writing to shadow mbr
>   block: sed-opal: check size of shadow mbr
> 
>  block/opal_proto.h            |  16 ++++
>  block/sed-opal.c              | 160 +++++++++++++++++++++++++++++++++-
>  include/linux/sed-opal.h      |   2 +
>  include/uapi/linux/sed-opal.h |  20 +++++
>  4 files changed, 196 insertions(+), 2 deletions(-)
> 

I'll review this over the weekend. Is this essentially the same thing
we reviewed a month or two ago or are there little differences due to
it be split across two different series?


> -- 
> 2.20.1
> 

Re: [PATCH 0/3] block: sed-opal: add support for shadow MBR done flag and write

[David Kozub]

On Thu, 2 May 2019, Scott Bauer wrote:

> On Wed, May 01, 2019 at 01:20:56AM +0200, David Kozub wrote:
>>
>> Jonas Rabenstein (3):
>>   block: sed-opal: add ioctl for done-mark of shadow mbr
>>   block: sed-opal: ioctl for writing to shadow mbr
>>   block: sed-opal: check size of shadow mbr
>>
>>  block/opal_proto.h            |  16 ++++
>>  block/sed-opal.c              | 160 +++++++++++++++++++++++++++++++++-
>>  include/linux/sed-opal.h      |   2 +
>>  include/uapi/linux/sed-opal.h |  20 +++++
>>  4 files changed, 196 insertions(+), 2 deletions(-)
>>
>
> I'll review this over the weekend. Is this essentially the same thing
> we reviewed a month or two ago or are there little differences due to
> it be split across two different series?

The first patch (block: sed-opal: add ioctl for done-mark of shadow mbr) 
is a bit different because a new struct and enum were introduced. The rest 
is pretty much the same. That's also why I kept the reviewd-by tags.

Best regards,
David