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..
