On Fri, Apr 28, 2023 at 4:59 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
>
> On Fri, Apr 28, 2023 at 4:57 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
> >
> > On 4/27/23 1:04 PM, Stanislav Fomichev wrote:
> > > Instead of assuming EFAULT, let's assume the BPF program's
> > > output is ignored.
> > >
> > > Remove "getsockopt: deny arbitrary ctx->retval" because it
> > > was actually testing optlen. We have separate set of tests
> > > for retval.
> > >
> > > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> > > ---
> > > .../selftests/bpf/prog_tests/sockopt.c | 80 +++++++++++++++++--
> > > 1 file changed, 74 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt.c b/tools/testing/selftests/bpf/prog_tests/sockopt.c
> > > index aa4debf62fc6..8dad30ce910e 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/sockopt.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt.c
> > > @@ -273,10 +273,30 @@ static struct sockopt_test {
> > > .error = EFAULT_GETSOCKOPT,
> > > },
> > > {
> > > - .descr = "getsockopt: deny arbitrary ctx->retval",
> > > + .descr = "getsockopt: ignore >PAGE_SIZE optlen",
> > > .insns = {
> > > - /* ctx->retval = 123 */
> > > - BPF_MOV64_IMM(BPF_REG_0, 123),
> > > + /* write 0xFF to the first optval byte */
> > > +
> > > + /* r6 = ctx->optval */
> > > + BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1,
> > > + offsetof(struct bpf_sockopt, optval)),
> > > + /* r2 = ctx->optval */
> > > + BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
> > > + /* r6 = ctx->optval + 1 */
> > > + BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, 1),
> > > +
> > > + /* r7 = ctx->optval_end */
> > > + BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_1,
> > > + offsetof(struct bpf_sockopt, optval_end)),
> > > +
> > > + /* if (ctx->optval + 1 <= ctx->optval_end) { */
> > > + BPF_JMP_REG(BPF_JGT, BPF_REG_6, BPF_REG_7, 1),
> > > + /* ctx->optval[0] = 0xF0 */
> > > + BPF_ST_MEM(BPF_B, BPF_REG_2, 0, 0xFF),
> > > + /* } */
> > > +
> > > + /* ctx->retval = 0 */
> > > + BPF_MOV64_IMM(BPF_REG_0, 0),
> > > BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
> > > offsetof(struct bpf_sockopt, retval)),
> > >
> > > @@ -287,9 +307,10 @@ static struct sockopt_test {
> > > .attach_type = BPF_CGROUP_GETSOCKOPT,
> > > .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
> > >
> > > - .get_optlen = 64,
> > > -
> > > - .error = EFAULT_GETSOCKOPT,
> > > + .get_level = 1234,
> > > + .get_optname = 5678,
> > > + .get_optval = {}, /* the changes are ignored */
> > > + .get_optlen = 4096 + 1,
> >
> > The patchset looks good. Thanks for taking care of it.
> >
> > One question, is it safe to the assume 4096 page size for all platforms in the
> > selftests?
>
> Good question; let me respin with sysconf() just to be safe..
Argh, the compiler yells at me:
error: initializer element is not a compile-time constant
I guess I'm just gonna do #define PAGE_SIZE 4096 and if we do hit some
problems on the other archs, I'll ifdef it in one place.
