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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).