From: Dan Carpenter <dan.carpenter@linaro.org>
To: dm-devel@lists.linux.dev
Cc: linux-hardening@vger.kernel.org, Kees Cook <keescook@chromium.org>
Subject: [bug report] dm ioctl: harden copy_params()'s copy_from_user() from malicious users
Date: Sun, 28 Apr 2024 16:12:07 +0300 [thread overview]
Message-ID: <f84a96d7-2df4-4f79-8c27-83a1d9574412@moroto.mountain> (raw)
Hi DM Maintainers and kernel hardenning people,
drivers/md/dm-ioctl.c
1931 static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kernel,
1932 int ioctl_flags, struct dm_ioctl **param, int *param_flags)
1933 {
1934 struct dm_ioctl *dmi;
1935 int secure_data;
1936 const size_t minimum_data_size = offsetof(struct dm_ioctl, data);
1937
1938 /* check_version() already copied version from userspace, avoid TOCTOU */
1939 if (copy_from_user((char *)param_kernel + sizeof(param_kernel->version),
1940 (char __user *)user + sizeof(param_kernel->version),
1941 minimum_data_size - sizeof(param_kernel->version)))
1942 return -EFAULT;
1943
1944 if (unlikely(param_kernel->data_size < minimum_data_size) ||
1945 unlikely(param_kernel->data_size > DM_MAX_TARGETS * DM_MAX_TARGET_PARAMS)) {
So what's happening here is that struct dm_ioctl->data[] is declared as
a 7 byte array, but it's actually a variable size array which could be
more or less than 7 bytes.
1946 DMERR("Invalid data size in the ioctl structure: %u",
1947 param_kernel->data_size);
1948 return -EINVAL;
1949 }
1950
1951 secure_data = param_kernel->flags & DM_SECURE_DATA_FLAG;
1952
1953 *param_flags = secure_data ? DM_WIPE_BUFFER : 0;
1954
1955 if (ioctl_flags & IOCTL_FLAGS_NO_PARAMS) {
1956 dmi = param_kernel;
1957 dmi->data_size = minimum_data_size;
1958 goto data_copied;
1959 }
1960
1961 /*
1962 * Use __GFP_HIGH to avoid low memory issues when a device is
1963 * suspended and the ioctl is needed to resume it.
1964 * Use kmalloc() rather than vmalloc() when we can.
1965 */
1966 dmi = NULL;
1967 dmi = kvmalloc(param_kernel->data_size, GFP_NOIO | __GFP_HIGH);
We allocate the correct size of the variable element array.
1968
1969 if (!dmi) {
1970 if (secure_data && clear_user(user, param_kernel->data_size))
1971 return -EFAULT;
1972 return -ENOMEM;
1973 }
1974
1975 *param_flags |= DM_PARAMS_MALLOC;
1976
1977 /* Copy from param_kernel (which was already copied from user) */
1978 memcpy(dmi, param_kernel, minimum_data_size);
1979
--> 1980 if (copy_from_user(&dmi->data, (char __user *)user + minimum_data_size,
1981 param_kernel->data_size - minimum_data_size))
Doesn't the kernel hardenning stuff have run time checks for if we
write beyond the end of a 7 byte array? Why not just declare it as a
zero element array?
1982 goto bad;
1983 data_copied:
1984 /* Wipe the user buffer so we do not return it to userspace */
1985 if (secure_data && clear_user(user, param_kernel->data_size))
1986 goto bad;
1987
1988 *param = dmi;
1989 return 0;
1990
1991 bad:
1992 free_params(dmi, param_kernel->data_size, *param_flags);
1993
1994 return -EFAULT;
1995 }
regards,
dan carpenter
next reply other threads:[~2024-04-28 13:12 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-28 13:12 Dan Carpenter [this message]
2024-04-28 16:20 ` [bug report] dm ioctl: harden copy_params()'s copy_from_user() from malicious users Kees Cook
2024-04-29 5:49 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f84a96d7-2df4-4f79-8c27-83a1d9574412@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=dm-devel@lists.linux.dev \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.