d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

netmap: fix copyin/copyout of nmreq options list

Browse Source 
The previous code unsuccesfully attempted to report a precise error for
each option in the user list. Moreover, commit 253b2ec199 broke some
ctrl-api-test (see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=260547).

With this patch we bail out as soon as an unrecoverable error is detected and
we properly check for copy boundaries. EOPNOTSUPP no longer immediately
returns an error, so that any other option in the list may be examined
by the caller code and a precise report of the (un)supported options can
be returned to the user.

With this patch, all ctrl-api-test unit tests pass again.

PR:			260547
Submitted by:		giuseppe.lettieri@unipi.it
Reviewed by:		vmaffione
MFC after:		14 days
This commit is contained in:
Vincenzo Maffione 2023-03-21 23:23:18 +00:00 committed by Oscar Zhao
parent 7fb1dd1ce6
commit 7c70ef3b4a
2 changed files with 65 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
sys/dev/netmap/netmap.c
View File

@ -3369,7 +3369,7 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
size_t rqsz, optsz, bufsz;
int error = 0;
char *ker = NULL, *p;
struct nmreq_option **next, *src, **opt_tab;
struct nmreq_option **next, *src, **opt_tab, *opt;
uint64_t *ptrs;
if (hdr->nr_reserved) {
@ -3420,24 +3420,36 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
*ptrs++ = hdr->nr_body;
*ptrs++ = hdr->nr_options;
p = (char *)ptrs;
/* copy the body */
error = copyin((void *)(uintptr_t)hdr->nr_body, p, rqsz);
if (error)
goto out_restore;
/* overwrite the user pointer with the in-kernel one */
hdr->nr_body = (uintptr_t)p;
/* prepare the options-list pointers and temporarily terminate
* the in-kernel list, in case we have to jump to out_restore
*/
next = (struct nmreq_option **)&hdr->nr_options;
src = *next;
hdr->nr_options = 0;
/* copy the body */
error = copyin(*(void **)ker, p, rqsz);
if (error)
goto out_restore;
p += rqsz;
/* start of the options table */
opt_tab = (struct nmreq_option **)p;
p += sizeof(opt_tab) * NETMAP_REQ_OPT_MAX;
/* copy the options */
next = (struct nmreq_option **)&hdr->nr_options;
src = *next;
while (src) {
struct nmreq_option *opt;
struct nmreq_option *nsrc;
if (p - ker + sizeof(uint64_t*) + sizeof(*src) > bufsz) {
error = EMSGSIZE;
/* there might be a loop in the list: don't try to
* copyout the options
*/
hdr->nr_options = 0;
goto out_restore;
}
/* copy the option header */
ptrs = (uint64_t *)p;
opt = (struct nmreq_option *)(ptrs + 1);
@ -3445,15 +3457,19 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
if (error)
goto out_restore;
rqsz += sizeof(*src);
p = (char *)(opt + 1);
/* make a copy of the user next pointer */
*ptrs = opt->nro_next;
/* overwrite the user pointer with the in-kernel one */
/* append the option to the in-kernel list */
*next = opt;
/* initialize the option as not supported.
* Recognized options will update this field.
/* temporarily teminate the in-kernel list, in case we have to
* jump to out_restore
*/
opt->nro_status = EOPNOTSUPP;
nsrc = (struct nmreq_option *)opt->nro_next;
opt->nro_next = 0;
opt->nro_status = 0;
/* check for invalid types */
if (opt->nro_reqtype < 1) {
@ -3461,12 +3477,11 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
nm_prinf("invalid option type: %u", opt->nro_reqtype);
opt->nro_status = EINVAL;
error = EINVAL;
goto next;
goto out_restore;
}
if (opt->nro_reqtype >= NETMAP_REQ_OPT_MAX) {
/* opt->nro_status is already EOPNOTSUPP */
error = EOPNOTSUPP;
/* opt->nro_status will be set to EOPNOTSUPP */
goto next;
}
@ -3479,12 +3494,10 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
opt->nro_status = EINVAL;
opt_tab[opt->nro_reqtype]->nro_status = EINVAL;
error = EINVAL;
goto next;
goto out_restore;
}
opt_tab[opt->nro_reqtype] = opt;
p = (char *)(opt + 1);
/* copy the option body */
optsz = nmreq_opt_size_by_type(opt->nro_reqtype,
opt->nro_size);
@ -3507,18 +3520,20 @@ nmreq_copyin(struct nmreq_header *hdr, int nr_body_is_user)
next:
/* move to next option */
next = (struct nmreq_option **)&opt->nro_next;
src = *next;
src = nsrc;
}
if (error)
nmreq_copyout(hdr, error);
return error;
/* initialize all the options as not supported. Recognized options
* will update their field.
*/
for (src = (struct nmreq_option *)hdr->nr_options; src;
src = (struct nmreq_option *)src->nro_next) {
src->nro_status = EOPNOTSUPP;
}
return 0;
out_restore:
ptrs = (uint64_t *)ker;
hdr->nr_body = *ptrs++;
hdr->nr_options = *ptrs++;
hdr->nr_reserved = 0;
nm_os_free(ker);
nmreq_copyout(hdr, error);
out_err:
return error;
}

23
tests/sys/netmap/ctrl-api-test.c
View File

@ -1012,9 +1012,10 @@ infinite_options(struct TestContext *ctx)
{
struct nmreq_option opt;
printf("Testing infinite list of options on %s\n", ctx->ifname_ext);
printf("Testing infinite list of options on %s (invalid options)\n", ctx->ifname_ext);
opt.nro_reqtype = 1234;
memset(&opt, 0, sizeof(opt));
opt.nro_reqtype = NETMAP_REQ_OPT_MAX + 1;
push_option(&opt, ctx);
opt.nro_next = (uintptr_t)&opt;
if (port_register_hwall(ctx) >= 0)
@ -1023,6 +1024,23 @@ infinite_options(struct TestContext *ctx)
return (errno == EMSGSIZE ? 0 : -1);
}
static int
infinite_options2(struct TestContext *ctx)
{
struct nmreq_option opt;
printf("Testing infinite list of options on %s (valid options)\n", ctx->ifname_ext);
memset(&opt, 0, sizeof(opt));
opt.nro_reqtype = NETMAP_REQ_OPT_OFFSETS;
push_option(&opt, ctx);
opt.nro_next = (uintptr_t)&opt;
if (port_register_hwall(ctx) >= 0)
return -1;
clear_options(ctx);
return (errno == EINVAL ? 0 : -1);
}
#ifdef CONFIG_NETMAP_EXTMEM
int
change_param(const char *pname, unsigned long newv, unsigned long *poldv)
@ -2049,6 +2067,7 @@ static struct mytest tests[] = {
decltest(vale_polling_enable_disable),
decltest(unsupported_option),
decltest(infinite_options),
decltest(infinite_options2),
#ifdef CONFIG_NETMAP_EXTMEM
decltest(extmem_option),
decltest(bad_extmem_option),