RE: [PATCH v1] mei: Don't encourage to use kernel internal types in user code

From: Winkler, Tomas
Date: Sat Feb 29 2020 - 11:28:29 EST



>
> uuid_le is internal kernel type which shall not be exposed to the user in the first
> place.
Why, these types are exported via include/uapi/linux/uuid.h

In order to mitigate the (wrong) distribution of the use of that type,
> switch MEI AMT sample to plain unsigned char array.

There was a change to guid_t from uuild_le, anyhow there is much more code
except this sample that uses those types.

Nack so far.
Tomas

>
> Note, there is no ABI change involved.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> samples/mei/mei-amt-version.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/samples/mei/mei-amt-version.c b/samples/mei/mei-amt-version.c
> index 32234481ad7d..458cb6db57c6 100644
> --- a/samples/mei/mei-amt-version.c
> +++ b/samples/mei/mei-amt-version.c
> @@ -90,7 +90,7 @@
> } while (0)
>
> struct mei {
> - uuid_le guid;
> + unsigned char guid[16];
> bool initialized;
> bool verbose;
> unsigned int buf_size;
> @@ -108,7 +108,7 @@ static void mei_deinit(struct mei *cl)
> cl->initialized = false;
> }
>
> -static bool mei_init(struct mei *me, const uuid_le *guid,
> +static bool mei_init(struct mei *me, const unsigned char *guid,
> unsigned char req_protocol_version, bool verbose) {
> int result;
> @@ -126,7 +126,7 @@ static bool mei_init(struct mei *me, const uuid_le
> *guid,
> memset(&data, 0, sizeof(data));
> me->initialized = true;
>
> - memcpy(&data.in_client_uuid, &me->guid, sizeof(me->guid));
> + memcpy(&data.in_client_uuid, me->guid, sizeof(me->guid));
> result = ioctl(me->fd, IOCTL_MEI_CONNECT_CLIENT, &data);
> if (result) {
> mei_err(me, "IOCTL_MEI_CONNECT_CLIENT receive message.
> err=%d\n", result); @@ -270,8 +270,11 @@ struct amt_host_if_resp_header {
> unsigned char data[0];
> } __attribute__((packed));
>
> -const uuid_le MEI_IAMTHIF = UUID_LE(0x12f80028, 0xb4b7, 0x4b2d, \
> - 0xac, 0xa8, 0x46, 0xe0, 0xff, 0x65, 0x81,
> 0x4c);
> +/* MEI AMT Interface GUID: 12f80028-b4b7-4b2d-aca8-46e0ff65814c */
> +const unsigned char mei_iamthif[16] = {
> + 0x28, 0x00, 0xf8, 0x12, 0xb7, 0xb4, 0x2d, 0x4b,
> + 0xac, 0xa8, 0x46, 0xe0, 0xff, 0x65, 0x81, 0x4c, };
>
> #define AMT_HOST_IF_CODE_VERSIONS_REQUEST 0x0400001A #define
> AMT_HOST_IF_CODE_VERSIONS_RESPONSE 0x0480001A @@ -295,7 +298,7
> @@ static bool amt_host_if_init(struct amt_host_if *acmd,
> unsigned long send_timeout, bool verbose) {
> acmd->send_timeout = (send_timeout) ? send_timeout : 20000;
> - acmd->initialized = mei_init(&acmd->mei_cl, &MEI_IAMTHIF, 0,
> verbose);
> + acmd->initialized = mei_init(&acmd->mei_cl, mei_iamthif, 0, verbose);
> return acmd->initialized;
> }
>
> --
> 2.25.0