Add an extended List Secrets IOCTL. In contrast to the first list IOCTL
this accepts an index as the first two bytes of the provided page as an
input. This index is then taken as the index offset for the list UVC to
receive later entries for the list. While at it fix some kernel doc
issues with the list function.
Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
arch/s390/include/uapi/asm/uvdevice.h | 4 ++
drivers/s390/char/uvdevice.c | 96 +++++++++++++++++++--------
2 files changed, 72 insertions(+), 28 deletions(-)
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
index 4947f26ad9fb..1cfa4e17af03 100644
--- a/arch/s390/include/uapi/asm/uvdevice.h
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -72,6 +72,7 @@ struct uvio_uvdev_info {
#define UVIO_ADD_SECRET_MAX_LEN 0x100000
#define UVIO_LIST_SECRETS_LEN 0x1000
#define UVIO_RETR_SECRET_MAX_LEN 0x2000
+#define UVIO_LIST_SECRETS_EXT_LEN UVIO_LIST_SECRETS_LEN
#define UVIO_DEVICE_NAME "uv"
#define UVIO_TYPE_UVC 'u'
@@ -83,6 +84,7 @@ enum UVIO_IOCTL_NR {
UVIO_IOCTL_LIST_SECRETS_NR,
UVIO_IOCTL_LOCK_SECRETS_NR,
UVIO_IOCTL_RETR_SECRET_NR,
+ UVIO_IOCTL_LIST_SECRETS_EXT_NR,
/* must be the last entry */
UVIO_IOCTL_NUM_IOCTLS
};
@@ -94,6 +96,7 @@ enum UVIO_IOCTL_NR {
#define UVIO_IOCTL_LIST_SECRETS UVIO_IOCTL(UVIO_IOCTL_LIST_SECRETS_NR)
#define UVIO_IOCTL_LOCK_SECRETS UVIO_IOCTL(UVIO_IOCTL_LOCK_SECRETS_NR)
#define UVIO_IOCTL_RETR_SECRET UVIO_IOCTL(UVIO_IOCTL_RETR_SECRET_NR)
+#define UVIO_IOCTL_LIST_SECRETS_EXT UVIO_IOCTL(UVIO_IOCTL_LIST_SECRETS_EXT_NR)
#define UVIO_SUPP_CALL(nr) (1ULL << (nr))
#define UVIO_SUPP_UDEV_INFO UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
@@ -102,5 +105,6 @@ enum UVIO_IOCTL_NR {
#define UVIO_SUPP_LIST_SECRETS UVIO_SUPP_CALL(UVIO_IOCTL_LIST_SECRETS_NR)
#define UVIO_SUPP_LOCK_SECRETS UVIO_SUPP_CALL(UVIO_IOCTL_LOCK_SECRETS_NR)
#define UVIO_SUPP_RETR_SECRET UVIO_SUPP_CALL(UVIO_IOCTL_RETR_SECRET_NR)
+#define UVIO_SUPP_LIST_SECRETS_EXT UVIO_SUPP_CALL(UVIO_IOCTL_LIST_SECRETS_EXT_NR)
#endif /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
index 1f90976293e8..69b0a69a4915 100644
--- a/drivers/s390/char/uvdevice.c
+++ b/drivers/s390/char/uvdevice.c
@@ -41,6 +41,7 @@ static const u32 ioctl_nr_to_uvc_bit[] __initconst = {
[UVIO_IOCTL_LIST_SECRETS_NR] = BIT_UVC_CMD_LIST_SECRETS,
[UVIO_IOCTL_LOCK_SECRETS_NR] = BIT_UVC_CMD_LOCK_SECRETS,
[UVIO_IOCTL_RETR_SECRET_NR] = BIT_UVC_CMD_RETR_ATTEST,
+ [UVIO_IOCTL_LIST_SECRETS_EXT_NR] = BIT_UVC_CMD_LIST_SECRETS,
};
static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
@@ -297,8 +298,39 @@ static int uvio_add_secret(struct uvio_ioctl_cb *uv_ioctl)
return ret;
}
-/** uvio_list_secrets() - perform a List Secret UVC
- * @uv_ioctl: ioctl control block
+/**
+ * The actual list(_ext) IOCTL.
+ * If list_ext is true, the first two bytes of the user buffer set the starting
+ * index of the list-UVC.
+ */
+static int list_secrets(struct uvio_ioctl_cb *uv_ioctl, bool list_ext)
+{
+ void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
+ u16 __user *user_index = (u16 __user *)uv_ioctl->argument_addr;
+ u16 start_idx = 0;
+ u8 *secrets;
+ int ret = 0;
+
+ BUILD_BUG_ON(UVIO_LIST_SECRETS_LEN != PAGE_SIZE);
+ if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
+ return -EINVAL;
+ /* The extended call accepts an u16 index as input */
+ if (list_ext && get_user(start_idx, user_index))
+ return -EFAULT;
+ secrets = (u8 *)get_zeroed_page(GFP_KERNEL);
+ if (!secrets)
+ return -ENOMEM;
+ uv_list_secrets(secrets, start_idx, &uv_ioctl->uv_rc, &uv_ioctl->uv_rrc);
+ if (copy_to_user(user_buf_arg, secrets, UVIO_LIST_SECRETS_LEN))
+ ret = -EFAULT;
+
+ free_pages((unsigned long)secrets, 0);
+ return ret;
+}
+
+/**
+ * uvio_list_secrets() - perform a List Secret UVC.
+ * @uv_ioctl: ioctl control block.
*
* uvio_list_secrets() performs the List Secret Ultravisor Call. It verifies
* that the given userspace argument address is valid and its size is sane.
@@ -312,37 +344,13 @@ static int uvio_add_secret(struct uvio_ioctl_cb *uv_ioctl)
* command rc. This won't be fenced in the driver and does not result in a
* negative return value.
*
- * Context: might sleep
+ * Context: might sleep.
*
* Return: 0 on success or a negative error code on error.
*/
static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
{
- void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
- struct uv_cb_guest_addr uvcb = {
- .header.len = sizeof(uvcb),
- .header.cmd = UVC_CMD_LIST_SECRETS,
- };
- void *secrets = NULL;
- int ret = 0;
-
- if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
- return -EINVAL;
-
- secrets = kvzalloc(UVIO_LIST_SECRETS_LEN, GFP_KERNEL);
- if (!secrets)
- return -ENOMEM;
-
- uvcb.addr = (u64)secrets;
- uv_call_sched(0, (u64)&uvcb);
- uv_ioctl->uv_rc = uvcb.header.rc;
- uv_ioctl->uv_rrc = uvcb.header.rrc;
-
- if (copy_to_user(user_buf_arg, secrets, UVIO_LIST_SECRETS_LEN))
- ret = -EFAULT;
-
- kvfree(secrets);
- return ret;
+ return list_secrets(uv_ioctl, false);
}
/** uvio_lock_secrets() - perform a Lock Secret Store UVC
@@ -432,6 +440,35 @@ static int uvio_retr_secret(struct uvio_ioctl_cb *uv_ioctl)
return ret;
}
+/**
+ * uvio_list_secrets_ext() - perform a List Secret UVC with a starting index.
+ * @uv_ioctl: ioctl control block.
+ *
+ * uvio_list_secrets_ext() performs the List Secret Ultravisor Call. It verifies
+ * that the given userspace argument address is valid and its size is sane.
+ * Every other check is made by the Ultravisor (UV) and won't result in a
+ * negative return value. It builds the request, performs the UV-call, and
+ * copies the result to userspace.
+ *
+ * The argument specifies the location for the result of the UV-Call.
+ * The first two bytes of the argument specify the starting index of the list.
+ * This should be zero for the first IOCTL. If UV reports more data (rc UVC_RC_MORE_DATA)
+ * another list_ext IOCTL with a higher starting index shows the following
+ * entries of the secret list.
+ *
+ * If the List Secrets UV facility is not present, UV will return invalid
+ * command rc. This won't be fenced in the driver and does not result in a
+ * negative return value.
+ *
+ * Context: might sleep.
+ *
+ * Return: 0 on success or a negative error code on error.
+ */
+static int uvio_list_secrets_ext(struct uvio_ioctl_cb *uv_ioctl)
+{
+ return list_secrets(uv_ioctl, true);
+}
+
static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
unsigned long cmd)
{
@@ -488,6 +525,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case UVIO_IOCTL_RETR_SECRET_NR:
ret = uvio_retr_secret(&uv_ioctl);
break;
+ case UVIO_IOCTL_LIST_SECRETS_EXT_NR:
+ ret = uvio_list_secrets_ext(&uv_ioctl);
+ break;
default:
ret = -ENOIOCTLCMD;
break;
--
2.43.0Hi Steffen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kvms390/next]
[also build test WARNING on s390/features linus/master v6.12-rc3 next-20241015]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Steffen-Eiden/s390-boot-uv-c-Use-a-constant-for-more-data-rc/20241015-193108
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git next
patch link: https://lore.kernel.org/r/20241015112859.3069210-6-seiden%40linux.ibm.com
patch subject: [PATCH v3 5/6] s390/uvdevice: Add List Secrets Ext IOCTL
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241016/202410161057.pV9QsHAq-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241016/202410161057.pV9QsHAq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410161057.pV9QsHAq-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/s390/char/uvdevice.c:302: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* The actual list(_ext) IOCTL.
vim +302 drivers/s390/char/uvdevice.c
300
301 /**
> 302 * The actual list(_ext) IOCTL.
303 * If list_ext is true, the first two bytes of the user buffer set the starting
304 * index of the list-UVC.
305 */
306 static int list_secrets(struct uvio_ioctl_cb *uv_ioctl, bool list_ext)
307 {
308 void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
309 u16 __user *user_index = (u16 __user *)uv_ioctl->argument_addr;
310 u16 start_idx = 0;
311 u8 *secrets;
312 int ret = 0;
313
314 BUILD_BUG_ON(UVIO_LIST_SECRETS_LEN != PAGE_SIZE);
315 if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
316 return -EINVAL;
317 /* The extended call accepts an u16 index as input */
318 if (list_ext && get_user(start_idx, user_index))
319 return -EFAULT;
320 secrets = (u8 *)get_zeroed_page(GFP_KERNEL);
321 if (!secrets)
322 return -ENOMEM;
323 uv_list_secrets(secrets, start_idx, &uv_ioctl->uv_rc, &uv_ioctl->uv_rrc);
324 if (copy_to_user(user_buf_arg, secrets, UVIO_LIST_SECRETS_LEN))
325 ret = -EFAULT;
326
327 free_pages((unsigned long)secrets, 0);
328 return ret;
329 }
330
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, Oct 15, 2024 at 01:28:58PM +0200, Steffen Eiden wrote: > Add an extended List Secrets IOCTL. In contrast to the first list IOCTL > this accepts an index as the first two bytes of the provided page as an > input. This index is then taken as the index offset for the list UVC to > receive later entries for the list. While at it fix some kernel doc > issues with the list function. > > Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com> > Signed-off-by: Steffen Eiden <seiden@linux.ibm.com> > --- > arch/s390/include/uapi/asm/uvdevice.h | 4 ++ > drivers/s390/char/uvdevice.c | 96 +++++++++++++++++++-------- > 2 files changed, 72 insertions(+), 28 deletions(-) ... > +/** > + * The actual list(_ext) IOCTL. > + * If list_ext is true, the first two bytes of the user buffer set the starting > + * index of the list-UVC. > + */ > +static int list_secrets(struct uvio_ioctl_cb *uv_ioctl, bool list_ext) This is not kernel-doc style :) > + free_pages((unsigned long)secrets, 0); > + return ret; free_page() instead of free_pages()?
On 10/15/24 2:01 PM, Heiko Carstens wrote: > On Tue, Oct 15, 2024 at 01:28:58PM +0200, Steffen Eiden wrote: >> Add an extended List Secrets IOCTL. In contrast to the first list IOCTL >> this accepts an index as the first two bytes of the provided page as an >> input. This index is then taken as the index offset for the list UVC to >> receive later entries for the list. While at it fix some kernel doc >> issues with the list function. >> >> Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com> >> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com> >> --- >> arch/s390/include/uapi/asm/uvdevice.h | 4 ++ >> drivers/s390/char/uvdevice.c | 96 +++++++++++++++++++-------- >> 2 files changed, 72 insertions(+), 28 deletions(-) > > ... > >> +/** >> + * The actual list(_ext) IOCTL. >> + * If list_ext is true, the first two bytes of the user buffer set the starting >> + * index of the list-UVC. >> + */ >> +static int list_secrets(struct uvio_ioctl_cb *uv_ioctl, bool list_ext) > > This is not kernel-doc style :) Thanks, this should not be kernel doc anyways (nor part of an API). Removing the additional star. > >> + free_pages((unsigned long)secrets, 0); >> + return ret; > > free_page() instead of free_pages()?
© 2016 - 2024 Red Hat, Inc.