From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D56ADE7719D for ; Fri, 10 Jan 2025 22:01:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BA0626B00A8; Fri, 10 Jan 2025 17:01:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AFEE26B00A9; Fri, 10 Jan 2025 17:01:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 903B66B00AF; Fri, 10 Jan 2025 17:01:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 672D36B00A8 for ; Fri, 10 Jan 2025 17:01:34 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 08E94120E9D for ; Fri, 10 Jan 2025 22:01:34 +0000 (UTC) X-FDA: 82992914508.16.630EE8C Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) by imf12.hostedemail.com (Postfix) with ESMTP id 1ACB24000A for ; Fri, 10 Jan 2025 22:01:31 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=afciTKLK; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.42 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736546492; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=1xVaG7BPrwWVqfXJ56Ko/Gx/Ry3hlHW50pgwCtZO10A=; b=N6uPiX0X+hLoKOC/zyv0zr4WWO0a2gkfp1hez9K0bHJ2L0TS9DFj+ntgQAxakLIDHiQCc6 xLPXO0zyIrkeoZKOg7DTpqY93aEABRz8re5J2rqYSuayZP1rcK+FVSqLmbY7WNgzqBlD3Q T9UPCMC4zyl9qT9TnErkyDfeMUwN1Fc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736546492; a=rsa-sha256; cv=none; b=fYZsTDZVfyq3WDngZN20t2BBB7TuAmeSaHsBgmfgauA6yfMdEuKQ7F9P1ahiUvx/Ygpsai zukMG6Kme8DUhgrIYLQFd/TOTB/MUg7wyU90ThqnH1fphjG6zoA05m5yKvdzVKD9AKkuQT o40QyNtDqvlg3xjHbydU94TJrvJzUPc= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=afciTKLK; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.42 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com Received: by mail-pj1-f42.google.com with SMTP id 98e67ed59e1d1-2ee9a780de4so3277120a91.3 for ; Fri, 10 Jan 2025 14:01:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736546491; x=1737151291; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=1xVaG7BPrwWVqfXJ56Ko/Gx/Ry3hlHW50pgwCtZO10A=; b=afciTKLKNOM6344RxnsyvwWqm15nyFgUiw+e6YPJyTkMx70hUKTe4iSIQEPQLrNktS f8eLn4AcazjUNMr0JX8G50nO5T3Pd3FNLDX2COs3iA4c3qjPH6lcDszMpf9CfI1SQDUw nEwA1XmLvLmGu+449PoMQbqZ0SdgV9LBnBVBrrdp82xdtx+ZvnhZD9aVrSMYgS+DrlX5 jmFk0C3Am0FZResank3DNkUnGcOFvW0FZ5PYs/JfoBURcOdNS8UEupivLFaIlwlHVBWh iElm61Kc6yP2fU1mQsqqf3w2OLqpP62XOZn/D1MTgrBLEEqmuAzalFH4SdBU2lWKXH7Z LaMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736546491; x=1737151291; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1xVaG7BPrwWVqfXJ56Ko/Gx/Ry3hlHW50pgwCtZO10A=; b=n6Wpk2ogud5lypepO+vu3/3wYbb3QFoivj+L07sPiIZfFYCr+OLOwZSxNlrFHLCCyx QIS3Nmtv/xMBQoXK8ihhlcR3pgiMphPxidZxg2UFgRXlneWkneJmRiGk8KPnZApKRkRM QGEvGTagmBHy7ldmUAo9sW1asNQruPk2i5DQUIx2PoQ9sDZjvR2PvoiChv/HLs47lO07 pm5F3HLRywqWtYOixRaWUijKWALNVGPbrsfUj+S+f2gvj5/qxR6qzCudS93PcqS8eY/R XMzcIv+fo/j3F6Eq9amPNIQNw6igQ84p91XVpC6d4JRU0GyIgjd58DPUxdD3PlfFq0V+ HJng== X-Forwarded-Encrypted: i=1; AJvYcCVZD9EFPCEXV/9IqFUoPjqcPNrhnTI/GICPPAFF/Q+fIJdIiFKR406l6aG69XGg5dpV8WGpUHQI+g==@kvack.org X-Gm-Message-State: AOJu0Yw0Zd3XRIbnjLVOH6wKk/zUlpmOdt2qAj/hpveX/WTibxxC7H3Q 3qztQGl36+mBSseiShkjjnNqrp1qtQroi7Z7SkbQcdRFUfAU2fdYbnSaInqBPlLGd4dKUpefGY/ Fn7VgFIO3/ldUy002ebbs3eeMNNA= X-Gm-Gg: ASbGncvhoK8Tt68ysMFmjjHkoAaBeZL3LzboqSttn5LEvbs5g/oxVz+Q/1rb67IxEax /cwkPlJ4hrX4+lR9duJizva06c1f+w8Ibk/xgKkhZ/DwaKxuRF5bg0w== X-Google-Smtp-Source: AGHT+IFA1D6dtouSmBLLDoBe3rsVJozlIk7ntkWozV0pB94F/MlCNeMV5QpP2vNf57S2uPnc4TMquKQKSeTWaWOffvI= X-Received: by 2002:a17:90b:540b:b0:2f2:3efd:96da with SMTP id 98e67ed59e1d1-2f548f4ea9emr19631765a91.24.1736546490666; Fri, 10 Jan 2025 14:01:30 -0800 (PST) MIME-Version: 1.0 References: <20250107020632.170883-1-linux@jordanrome.com> <20250107020632.170883-2-linux@jordanrome.com> In-Reply-To: <20250107020632.170883-2-linux@jordanrome.com> From: Andrii Nakryiko Date: Fri, 10 Jan 2025 14:01:18 -0800 X-Gm-Features: AbW1kvY1FFGBVFDb8cTCIBl-8iqmqwC__rSKToTFCUZAFx3YWKxLdBaqzxIYCK4 Message-ID: Subject: Re: [bpf-next v2 2/2] selftests/bpf: Add tests for bpf_copy_from_user_task_str To: Jordan Rome Cc: bpf@vger.kernel.org, linux-mm@kvack.org, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Kernel Team , Andrew Morton , Shakeel Butt Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: twu3mtxcpu8gqu7zai6xwjmrcq5f89zk X-Rspamd-Queue-Id: 1ACB24000A X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1736546491-114992 X-HE-Meta: U2FsdGVkX18G5Zeh2e/JrM1pwJBo5ugtAU6ftZ2CR2k/cpB/bDCO+YtNaLkWRbByag9pwZnzKMzQNfxg5fim6II4u9U9kdOiC3htoBNzld3XsV5nYsPK4ka1UfXCQkoEdNRwET60/YiOMo8kY+GDCKL6M2PQ3sllT3hVJHoYG08CHc+L4mIbIqTrPwkB3HjtpMKTzNNy2snAbWfVXpSEEGRof5cbbJc+AZYPWTJtu6mO+qEmSjAM4hgxmbKpxffdkKb36Cz6eN5dzD4+KPBIT9S4/OUQsjTdWhCzXI5LXnF7/bFl7YQXtUbEpoCjxocNssvNJecEIiWyzC+SS85uoWPWpBJrUmKUlyzYed3g8RhqmpvuYxUDn4eZdT28+h9bDyUF80VOQVZZk3yt9f6tKPlW8oiSPRZzqnWNbvuyAiPM5cSpcdBdiqjArwHFAPtomMXB4ECxxsRHszCKbA/pknIgSzBrTi8XsdKGkrRifXkkYSK3uUGFXlEP7m2paoSGHxH7k/8Q9x0Awx2zmCKCgu+iYctbxrFxu2IvsWvepnhpHN6C4kliHzd6gVNQHjaBiC/WYWBgocnwrMovZiqQOS1GCxZWP4RooPbQhER7xIghz4p+ezJU0I7VG2bVBnlu4LPyDvKxBNtLCVqifFeUkvplRG9K5NYedI0Hk9vjy7tJT9U9GsvuOaMux80f0A6Y+cx322uRY0RGbYDNYH0ec2b6+csfxa3YkwL61cXL3ovFx86aRSwKUyKnlaHppCyKbcTZSXyS0FF2x4uzaMjOXOhCQPDhCMWYBwS93t8lg73ls5M9AM4jOOmtOI6UPhOzvkxJyy9voIVw7cCDGe1OMPW0sUSwL5cuFek5k6rWQC5CykpWcICjzCUwHDLktbtkSFtSg6t2ZeyinCYDhGGGLWQbSUntZwcx6Mlecnd6nIqOStVezVBVwFqJMQtn7uGi3uuEwpKluJBQkFBrSuT Noza/lsd rEaat2gLF6XByozWLwIv946XKaK26smX6kFixiDPHcRpXRYrIQXsUJctUo5zdWtOfUERvreDdvgufSBcph8tIm5nBpATE8a4yVfbioqgWpxJvaCwQxGYiJ7dr3VmLrw9lWyTdETiNqw2F+KuUQJQZZ8HRAQa6oCOmPdAwqM1lbkU0R3BW22taOQVmBzPmwGxOSSvmfPLQMQWFEF4RGQGex2pNlxCqP426rvv2w+tEpo154Me/brNWPq5ZBRgGJ2rA2+bQLpvtyKQSTNIteI8T2/xhsfNhDUK+/ft+j33wX0apwdkPndk9vufXMJ9Yc9JtOUcfRxksYo2JPF/tE6O3iZG6zwEyRGxGIlLdSC09zMIDGKRTsS/P5YI0Nh0CLr0rVD5vF54MBYbmesw04tELoK8VXykcq4yeq/qZhzXDA0aEBvPWOb0nZK2JuGbv0+SgJyxsPdBEbFCYg+8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Jan 6, 2025 at 6:20=E2=80=AFPM Jordan Rome w= rote: > > This adds tests for both the happy path and the > error path (with and without the BPF_F_PAD_ZEROS flag). > > Signed-off-by: Jordan Rome > --- > .../selftests/bpf/prog_tests/bpf_iter.c | 7 +++ > .../selftests/bpf/prog_tests/read_vsyscall.c | 1 + > .../selftests/bpf/progs/bpf_iter_tasks.c | 55 +++++++++++++++++++ > .../selftests/bpf/progs/read_vsyscall.c | 6 +- > 4 files changed, 67 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/te= sting/selftests/bpf/prog_tests/bpf_iter.c > index 6f1bfacd7375..8ed864793bd1 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > @@ -34,6 +34,8 @@ > #include "bpf_iter_ksym.skel.h" > #include "bpf_iter_sockmap.skel.h" > > +static char test_data[] =3D "test_data"; > + > static void test_btf_id_or_null(void) > { > struct bpf_iter_test_kern3 *skel; > @@ -328,12 +330,17 @@ static void test_task_sleepable(void) > if (!ASSERT_OK_PTR(skel, "bpf_iter_tasks__open_and_load")) > return; > > + skel->bss->user_ptr =3D test_data; > do_dummy_read(skel->progs.dump_task_sleepable); > > ASSERT_GT(skel->bss->num_expected_failure_copy_from_user_task, 0, > "num_expected_failure_copy_from_user_task"); > ASSERT_GT(skel->bss->num_success_copy_from_user_task, 0, > "num_success_copy_from_user_task"); > + ASSERT_GT(skel->bss->num_expected_failure_copy_from_user_task_str= , 0, > + "num_expected_failure_copy_from_user_task_str"); > + ASSERT_GT(skel->bss->num_success_copy_from_user_task_str, 0, > + "num_success_copy_from_user_task_str"); > > bpf_iter_tasks__destroy(skel); > } > diff --git a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c b/too= ls/testing/selftests/bpf/prog_tests/read_vsyscall.c > index c7b9ba8b1d06..a8d1eaa67020 100644 > --- a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c > +++ b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c > @@ -24,6 +24,7 @@ struct read_ret_desc { > { .name =3D "copy_from_user", .ret =3D -EFAULT }, > { .name =3D "copy_from_user_task", .ret =3D -EFAULT }, > { .name =3D "copy_from_user_str", .ret =3D -EFAULT }, > + { .name =3D "copy_from_user_task_str", .ret =3D -EFAULT }, > }; > > void test_read_vsyscall(void) > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c b/tools/t= esting/selftests/bpf/progs/bpf_iter_tasks.c > index bc10c4e4b4fa..90691e34b915 100644 > --- a/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_tasks.c > @@ -9,6 +9,7 @@ char _license[] SEC("license") =3D "GPL"; > uint32_t tid =3D 0; > int num_unknown_tid =3D 0; > int num_known_tid =3D 0; > +void *user_ptr =3D 0; > > SEC("iter/task") > int dump_task(struct bpf_iter__task *ctx) > @@ -35,7 +36,9 @@ int dump_task(struct bpf_iter__task *ctx) > } > > int num_expected_failure_copy_from_user_task =3D 0; > +int num_expected_failure_copy_from_user_task_str =3D 0; > int num_success_copy_from_user_task =3D 0; > +int num_success_copy_from_user_task_str =3D 0; > > SEC("iter.s/task") > int dump_task_sleepable(struct bpf_iter__task *ctx) > @@ -44,6 +47,9 @@ int dump_task_sleepable(struct bpf_iter__task *ctx) > struct task_struct *task =3D ctx->task; > static const char info[] =3D " =3D=3D=3D END =3D=3D=3D"; > struct pt_regs *regs; > + char task_str1[10] =3D "aaaaaaaaaa"; > + char task_str2[10], task_str3[10]; > + char task_str4[20] =3D "aaaaaaaaaaaaaaaaaaaa"; > void *ptr; > uint32_t user_data =3D 0; > int ret; > @@ -78,8 +84,57 @@ int dump_task_sleepable(struct bpf_iter__task *ctx) > BPF_SEQ_PRINTF(seq, "%s\n", info); > return 0; > } > + > ++num_success_copy_from_user_task; > > + /* Read an invalid pointer and ensure we get an error */ > + ptr =3D NULL; > + ret =3D bpf_copy_from_user_task_str((char *)task_str1, sizeof(tas= k_str1), ptr, task, 0); > + if (ret >=3D 0 || task_str1[9] !=3D 'a') { > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; > + } > + > + /* Read an invalid pointer and ensure we get error with pad zeros= flag */ > + ptr =3D NULL; > + ret =3D bpf_copy_from_user_task_str((char *)task_str1, sizeof(tas= k_str1), ptr, task, BPF_F_PAD_ZEROS); > + if (ret >=3D 0 || task_str1[9] !=3D '\0') { > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; > + } > + > + ++num_expected_failure_copy_from_user_task_str; > + > + /* Same length as the string */ > + ret =3D bpf_copy_from_user_task_str((char *)task_str2, 10, user_p= tr, task, 0); > + if (bpf_strncmp(task_str2, 10, "test_data\0") !=3D 0 || ret !=3D = 10) { > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; > + } > + > + /* Shorter length than the string */ > + ret =3D bpf_copy_from_user_task_str((char *)task_str3, 9, user_pt= r, task, 0); > + if (bpf_strncmp(task_str3, 9, "test_dat\0") !=3D 0 || ret !=3D 9)= { > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; > + } > + > + /* Longer length than the string */ > + ret =3D bpf_copy_from_user_task_str((char *)task_str4, 20, user_p= tr, task, 0); > + if (bpf_strncmp(task_str4, 10, "test_data\0") !=3D 0 || ret !=3D = 10 || task_str4[sizeof(task_str4) - 1] !=3D 'a') { > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; > + } > + > + /* Longer length than the string with pad zeros flag */ > + ret =3D bpf_copy_from_user_task_str((char *)task_str4, 20, user_p= tr, task, BPF_F_PAD_ZEROS); > + if (bpf_strncmp(task_str4, 10, "test_data\0") !=3D 0 || ret !=3D = 10 || task_str4[sizeof(task_str4) - 1] !=3D '\0') { looks like a long string, please check 100 character limit > + BPF_SEQ_PRINTF(seq, "%s\n", info); > + return 0; nit: this BPF_SEQ_PRINTF() + return 0 repetition is... repetitive :) `goto drop_out;` maybe? > + } > + > + ++num_success_copy_from_user_task_str; > + > if (ctx->meta->seq_num =3D=3D 0) > BPF_SEQ_PRINTF(seq, " tgid gid data\n"); > > diff --git a/tools/testing/selftests/bpf/progs/read_vsyscall.c b/tools/te= sting/selftests/bpf/progs/read_vsyscall.c > index 39ebef430059..623c1c5bd2d0 100644 > --- a/tools/testing/selftests/bpf/progs/read_vsyscall.c > +++ b/tools/testing/selftests/bpf/progs/read_vsyscall.c > @@ -8,14 +8,15 @@ > > int target_pid =3D 0; > void *user_ptr =3D 0; > -int read_ret[9]; > +int read_ret[10]; > > char _license[] SEC("license") =3D "GPL"; > > /* > - * This is the only kfunc, the others are helpers > + * These are the kfuncs, the others are helpers > */ > int bpf_copy_from_user_str(void *dst, u32, const void *, u64) __weak __k= sym; > +int bpf_copy_from_user_task_str(void *dst, u32, const void *, struct tas= k_struct *, u64) __weak __ksym; these definitions should be coming from vmlinux.h, no need to add them manually anymore > > SEC("fentry/" SYS_PREFIX "sys_nanosleep") > int do_probe_read(void *ctx) > @@ -47,6 +48,7 @@ int do_copy_from_user(void *ctx) > read_ret[7] =3D bpf_copy_from_user_task(buf, sizeof(buf), user_pt= r, > bpf_get_current_task_btf(),= 0); > read_ret[8] =3D bpf_copy_from_user_str((char *)buf, sizeof(buf), = user_ptr, 0); > + read_ret[9] =3D bpf_copy_from_user_task_str((char *)buf, sizeof(b= uf), user_ptr, bpf_get_current_task_btf(), 0); please drop all those (char *) casts, in C any pointer is implicitly castable to `void *` and `void *` is (implicitly) castable to any other pointer. C++ is stricter, but in C it's canonical to not do casting > > return 0; > } > -- > 2.43.5 >