* [PATCH net,stable] qmi_wwan: Fix out-of-bounds read
From: Bjørn Mork @ 2019-06-24 16:45 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Hillf Danton, Bjørn Mork, Kristian Evensen
The syzbot reported
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x67/0x231 mm/kasan/report.c:188
__kasan_report.cold+0x1a/0x32 mm/kasan/report.c:317
kasan_report+0xe/0x20 mm/kasan/common.c:614
qmi_wwan_probe+0x342/0x360 drivers/net/usb/qmi_wwan.c:1417
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x660 drivers/base/dd.c:509
driver_probe_device+0x104/0x210 drivers/base/dd.c:670
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:777
bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
Caused by too many confusing indirections and casts.
id->driver_info is a pointer stored in a long. We want the
pointer here, not the address of it.
Thanks-to: Hillf Danton <hdanton@sina.com>
Reported-by: syzbot+b68605d7fadd21510de1@syzkaller.appspotmail.com
Cc: Kristian Evensen <kristian.evensen@gmail.com>
Fixes: e4bf63482c30 ("qmi_wwan: Add quirk for Quectel dynamic config")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
The bug was introduced in v5.2-rc1 but has been backported to stable kernels.
So this fix also needs to go into stable.
drivers/net/usb/qmi_wwan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d080f8048e52..8b4ad10cf940 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1482,7 +1482,7 @@ static int qmi_wwan_probe(struct usb_interface *intf,
* different. Ignore the current interface if the number of endpoints
* equals the number for the diag interface (two).
*/
- info = (void *)&id->driver_info;
+ info = (void *)id->driver_info;
if (info->data & QMI_WWAN_QUIRK_QUECTEL_DYNCFG) {
if (desc->bNumEndpoints == 2)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH bpf-next v5 1/3] devmap/cpumap: Use flush list instead of bitmap
From: Jonathan Lemon @ 2019-06-24 16:43 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller
In-Reply-To: <156125626115.5209.3880071777007082264.stgit@alrua-x1>
On 22 Jun 2019, at 19:17, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The socket map uses a linked list instead of a bitmap to keep track of
> which entries to flush. Do the same for devmap and cpumap, as this means we
> don't have to care about the map index when enqueueing things into the
> map (and so we can cache the map lookup).
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: add xsk_ring_prod__free() function
From: Andrii Nakryiko @ 2019-06-24 16:42 UTC (permalink / raw)
To: Eelco Chaudron
Cc: Karlsson, Magnus, Networking, Alexei Starovoitov, Daniel Borkmann,
Martin Lau, Song Liu, Yonghong Song
In-Reply-To: <1C59E98E-7F4B-4FCA-AB95-68D3819C489C@redhat.com>
On Mon, Jun 24, 2019 at 2:37 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
>
>
> On 21 Jun 2019, at 21:13, Andrii Nakryiko wrote:
>
> > On Fri, Jun 21, 2019 at 8:26 AM Eelco Chaudron <echaudro@redhat.com>
> > wrote:
> >>
> >> When an AF_XDP application received X packets, it does not mean X
> >> frames can be stuffed into the producer ring. To make it easier for
> >> AF_XDP applications this API allows them to check how many frames can
> >> be added into the ring.
> >>
> >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >> ---
> >> tools/lib/bpf/xsk.h | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> >> index 82ea71a0f3ec..86f3d485e957 100644
> >> --- a/tools/lib/bpf/xsk.h
> >> +++ b/tools/lib/bpf/xsk.h
> >> @@ -95,6 +95,12 @@ static inline __u32 xsk_prod_nb_free(struct
> >> xsk_ring_prod *r, __u32 nb)
> >> return r->cached_cons - r->cached_prod;
> >> }
> >>
> >> +static inline __u32 xsk_ring_prod__free(struct xsk_ring_prod *r)
> >
> > This is a very bad name choice. __free is used for functions that free
> > memory and resources. One function below I see avail is used in the
> > name, why not xsk_ring_prog__avail?
>
> Must agree that free sound like you are freeing entries… However, I
> just kept the naming already in the API/file (see above,
> xsk_prod_nb_free()).
> Reading the code there is a difference as xx_avail() means available
> filled entries, where xx_free() means available free entries.
>
> So I could rename it to xsk_ring_prod__nb_free() maybe?
I'm fine with __nb_free, yes. Thanks!
>
> Forgot to include Magnus in the email, so copied him in, for some
> comments.
>
> >> +{
> >> + r->cached_cons = *r->consumer + r->size;
> >> + return r->cached_cons - r->cached_prod;
> >> +}
> >> +
> >> static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32
> >> nb)
> >> {
> >> __u32 entries = r->cached_prod - r->cached_cons;
> >> --
> >> 2.20.1
> >>
^ permalink raw reply
* Re: [PATCH bpf-next v5 3/3] devmap: Allow map lookups from eBPF
From: Jonathan Lemon @ 2019-06-24 16:41 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller
In-Reply-To: <156125626158.5209.14261186257358121509.stgit@alrua-x1>
On 22 Jun 2019, at 19:17, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> We don't currently allow lookups into a devmap from eBPF, because the map
> lookup returns a pointer directly to the dev->ifindex, which shouldn't be
> modifiable from eBPF.
>
> However, being able to do lookups in devmaps is useful to know (e.g.)
> whether forwarding to a specific interface is enabled. Currently, programs
> work around this by keeping a shadow map of another type which indicates
> whether a map index is valid.
>
> Since we now have a flag to make maps read-only from the eBPF side, we can
> simply lift the lookup restriction if we make sure this flag is always set.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Arnd Bergmann @ 2019-06-24 16:40 UTC (permalink / raw)
To: Alex Elder
Cc: Johannes Berg, Dan Williams, Subash Abhinov Kasiviswanathan,
abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
DTML, Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM,
linux-arm-msm, Linux Kernel Mailing List, linux-soc, Networking,
syadagir
In-Reply-To: <db34aa39-6cf1-4844-1bfe-528e391c3729@linaro.org>
On Mon, Jun 24, 2019 at 6:21 PM Alex Elder <elder@linaro.org> wrote:
> On 6/18/19 2:03 PM, Johannes Berg wrote:
>
> > Really there are two possible ways (and they intersect to some extent).
> >
> > One is the whole multi-function device, where a single WWAN device is
> > composed of channels offered by actually different drivers, e.g. for a
> > typical USB device you might have something like cdc_ether and the
> > usb_wwan TTY driver. In this way, we need to "compose" the WWAN device
> > similarly, e.g. by using the underlying USB device "struct device"
> > pointer to tie it together.
>
> I *think* this model makes the most sense. But at this point
> it would take very little to convince me otherwise... (And then
> I saw Arnd's message advocating the other one, unfortunately...)
>
> > The other is something like IPA or the Intel modem driver, where the
> > device is actually a single (e.g. PCIe) device and just has a single
> > driver, but that single driver offers different channels.
>
> What I don't like about this is that it's more monolithic. It
> seems better to have the low-level IPA or Intel modem driver (or
> any other driver that can support communication between the AP
> and WWAN device) present communication paths that other function-
> specific drivers can attach to and use.
I did not understand Johannes description as two competing models
for the same code, but rather two kinds of existing hardware that
a new driver system would have to deal with.
I was trying to simplify it to just having the second model, by adding
a hack to support the first, but my view was rather unpopular so
far, so if everyone agrees on one way to do it, don't worry about me ;-)
> > Now, it's not clear to me where IPA actually falls, because so far we've
> > been talking about the IPA driver only as providing *netdevs*, not any
> > control channels, so I'm not actually sure where the control channel is.
>
> There is user space code that handles all of this, and as far as I
> can tell, parts of it will always remain proprietary.
Two replies on this:
- to answer Johannes question, my understanding is that the interface
between kernel and firmware/hardware for IPA has a single 'struct
device' that is used for both the data and the control channels,
rather than having a data channel and an independent control device,
so this falls into the same category as the Intel one (please correct
me on that)
- The user space being proprietary is exactly what we need to avoid
with the wwan subsystem. We need to be able to use the same
method for setting up Intel, Qualcomm, Samsung, Unisoc or
Hisilicon modems or anything else that hooks into the subsystem,
and support that in network manager as well as the Android
equivalent.
If Qualcomm wants to provide their own proprietary user space
solution, we can't stop them, but then that should also work on
all the others unless they intentionally break it. ;-)
> > and simply require that the channel is attached to the wwan device with
> > the representation-specific call (wwan_attach_netdev, wwan_attach_tty,
> > ...).
>
> Or maybe have the WWAN device present interfaces with attributes,
> and have drivers that are appropriate for each interface attach
> to only the ones they recognize they support.
I think you both mean the same thing here, a structure with callback
pointers that may or may not be filled by the driver depending on its
capabilities.
What we should try to avoid though is a way to add driver private
interfaces that risk having multiple drivers create similar functionality
in incompatible ways.
Arnd
^ permalink raw reply
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Jonathan Lemon @ 2019-06-24 16:41 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller
In-Reply-To: <156125626136.5209.14349225282974871197.stgit@alrua-x1>
On 22 Jun 2019, at 19:17, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The bpf_redirect_map() helper used by XDP programs doesn't return any
> indication of whether it can successfully redirect to the map index it was
> given. Instead, BPF programs have to track this themselves, leading to
> programs using duplicate maps to track which entries are populated in the
> devmap.
>
> This patch fixes this by moving the map lookup into the bpf_redirect_map()
> helper, which makes it possible to return failure to the eBPF program. The
> lower bits of the flags argument is used as the return code, which means
> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>
> With this, a BPF program can check the return code from the helper call and
> react by, for instance, substituting a different redirect. This works for
> any type of map used for redirect.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next v5 0/3] xdp: Allow lookup into devmaps before redirect
From: Andrii Nakryiko @ 2019-06-24 16:38 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Networking, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller, Jonathan Lemon
In-Reply-To: <156125626076.5209.13424524054109901554.stgit@alrua-x1>
On Sat, Jun 22, 2019 at 7:19 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> When using the bpf_redirect_map() helper to redirect packets from XDP, the eBPF
> program cannot currently know whether the redirect will succeed, which makes it
> impossible to gracefully handle errors. To properly fix this will probably
> require deeper changes to the way TX resources are allocated, but one thing that
> is fairly straight forward to fix is to allow lookups into devmaps, so programs
> can at least know when a redirect is *guaranteed* to fail because there is no
> entry in the map. Currently, programs work around this by keeping a shadow map
> of another type which indicates whether a map index is valid.
>
> This series contains two changes that are complementary ways to fix this issue:
>
> - Moving the map lookup into the bpf_redirect_map() helper (and caching the
> result), so the helper can return an error if no value is found in the map.
> This includes a refactoring of the devmap and cpumap code to not care about
> the index on enqueue.
>
> - Allowing regular lookups into devmaps from eBPF programs, using the read-only
> flag to make sure they don't change the values.
>
> The performance impact of the series is negligible, in the sense that I cannot
> measure it because the variance between test runs is higher than the difference
> pre/post series.
>
> Changelog:
>
> v5:
> - Rebase on latest bpf-next.
> - Update documentation for bpf_redirect_map() with the new meaning of flags.
>
> v4:
> - Fix a few nits from Andrii
> - Lose the #defines in bpf.h and just compare the flags argument directly to
> XDP_TX in bpf_xdp_redirect_map().
>
> v3:
> - Adopt Jonathan's idea of using the lower two bits of the flag value as the
> return code.
> - Always do the lookup, and cache the result for use in xdp_do_redirect(); to
> achieve this, refactor the devmap and cpumap code to get rid the bitmap for
> selecting which devices to flush.
>
> v2:
> - For patch 1, make it clear that the change works for any map type.
> - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the return
> value read-only.
>
> ---
>
> Toke Høiland-Jørgensen (3):
> devmap/cpumap: Use flush list instead of bitmap
> bpf_xdp_redirect_map: Perform map lookup in eBPF helper
> devmap: Allow map lookups from eBPF
>
>
> include/linux/filter.h | 1
> include/uapi/linux/bpf.h | 7 ++-
> kernel/bpf/cpumap.c | 106 ++++++++++++++++++++-----------------------
> kernel/bpf/devmap.c | 113 ++++++++++++++++++++++------------------------
> kernel/bpf/verifier.c | 7 +--
> net/core/filter.c | 29 +++++-------
> 6 files changed, 123 insertions(+), 140 deletions(-)
>
Looks like you forgot to add my Acked-by's for your patches?
^ permalink raw reply
* WWAN Controller Framework (was IPA [PATCH v2 00/17])
From: Alex Elder @ 2019-06-24 16:30 UTC (permalink / raw)
To: davem, arnd, bjorn.andersson, ilias.apalodimas
Cc: evgreen, benchan, ejcaruso, cpratapa, syadagir, subashab,
abhishek.esse, netdev, devicetree, linux-kernel, linux-soc,
linux-arm-kernel, linux-arm-msm
In-Reply-To: <20190531035348.7194-1-elder@linaro.org>
OK I want to try to organize a little more concisely some of the
discussion on this, because there is a very large amount of volume
to date and I think we need to try to narrow the focus back down
again.
I'm going to use a few terms here. Some of these I really don't
like, but I want to be unambiguous *and* (at least for now) I want
to avoid the very overloaded term "device".
I have lots more to say, but let's start with a top-level picture,
to make sure we're all on the same page.
WWAN Communication
Channel (Physical)
| ------------------------
------------ v | :+ Control | \
| |-----------| :+ Data | |
| AP | | WWAN unit :+ Voice | > Functions
| |===========| :+ GPS | |
------------ ^ | :+ ... | /
| -------------------------
Multiplexed WWAN
Communication
Channel (Physical)
- The *AP* is the main CPU complex that's running Linux on one or
more CPU cores.
- A *WWAN unit* is an entity that shares one or more physical
*WWAN communication channels* with the AP.
- A *WWAN communication channel* is a bidirectional means of
carrying data between the AP and WWAN unit.
- A WWAN communication channel carries data using a *WWAN protocol*.
- A WWAN unit implements one or more *WWAN functions*, such as
5G data, LTE voice, GPS, and so on.
- A WWAN unit shall implement a *WWAN control function*, used to
manage the use of other WWAN functions, as well as the WWAN unit
itself.
- The AP communicates with a WWAN function using a WWAN protocol.
- A WWAN physical channel can be *multiplexed*, in which case it
carries the data for one or more *WWAN logical channels*.
- A multiplexed WWAN communication channel uses a *WWAN wultiplexing
protocol*, which is used to separate independent data streams
carrying other WWAN protocols.
- A WWAN logical channel carries a bidirectional stream of WWAN
protocol data between an entity on the AP and a WWAN function.
Does that adequately represent a very high-level picture of what
we're trying to manage?
And if I understand it right, the purpose of the generic framework
being discussed is to define a common mechanism for managing (i.e.,
discovering, creating, destroying, querying, configuring, enabling,
disabling, etc.) WWAN units and the functions they implement, along
with the communication and logical channels used to communicate with
them.
Comments?
-Alex
^ permalink raw reply
* [PATCHv2 net] tipc: check msg->req data len in tipc_nl_compat_bearer_disable
From: Xin Long @ 2019-06-24 16:28 UTC (permalink / raw)
To: network dev
Cc: davem, Eric Dumazet, Jon Maloy, Ying Xue, tipc-discussion,
syzkaller-bugs
This patch is to fix an uninit-value issue, reported by syzbot:
BUG: KMSAN: uninit-value in memchr+0xce/0x110 lib/string.c:981
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x191/0x1f0 lib/dump_stack.c:113
kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
__msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
memchr+0xce/0x110 lib/string.c:981
string_is_valid net/tipc/netlink_compat.c:176 [inline]
tipc_nl_compat_bearer_disable+0x2a1/0x480 net/tipc/netlink_compat.c:449
__tipc_nl_compat_doit net/tipc/netlink_compat.c:327 [inline]
tipc_nl_compat_doit+0x3ac/0xb00 net/tipc/netlink_compat.c:360
tipc_nl_compat_handle net/tipc/netlink_compat.c:1178 [inline]
tipc_nl_compat_recv+0x1b1b/0x27b0 net/tipc/netlink_compat.c:1281
TLV_GET_DATA_LEN() may return a negtive int value, which will be
used as size_t (becoming a big unsigned long) passed into memchr,
cause this issue.
Similar to what it does in tipc_nl_compat_bearer_enable(), this
fix is to return -EINVAL when TLV_GET_DATA_LEN() is negtive in
tipc_nl_compat_bearer_disable(), as well as in
tipc_nl_compat_link_stat_dump() and tipc_nl_compat_link_reset_stats().
v1->v2:
- add the missing Fixes tags per Eric's request.
Fixes: 0762216c0ad2 ("tipc: fix uninit-value in tipc_nl_compat_bearer_enable")
Fixes: 8b66fee7f8ee ("tipc: fix uninit-value in tipc_nl_compat_link_reset_stats")
Reported-by: syzbot+30eaa8bf392f7fafffaf@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/netlink_compat.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index c6a04c0..cf15506 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -445,7 +445,11 @@ static int tipc_nl_compat_bearer_disable(struct tipc_nl_compat_cmd_doit *cmd,
if (!bearer)
return -EMSGSIZE;
- len = min_t(int, TLV_GET_DATA_LEN(msg->req), TIPC_MAX_BEARER_NAME);
+ len = TLV_GET_DATA_LEN(msg->req);
+ if (len <= 0)
+ return -EINVAL;
+
+ len = min_t(int, len, TIPC_MAX_BEARER_NAME);
if (!string_is_valid(name, len))
return -EINVAL;
@@ -539,7 +543,11 @@ static int tipc_nl_compat_link_stat_dump(struct tipc_nl_compat_msg *msg,
name = (char *)TLV_DATA(msg->req);
- len = min_t(int, TLV_GET_DATA_LEN(msg->req), TIPC_MAX_LINK_NAME);
+ len = TLV_GET_DATA_LEN(msg->req);
+ if (len <= 0)
+ return -EINVAL;
+
+ len = min_t(int, len, TIPC_MAX_BEARER_NAME);
if (!string_is_valid(name, len))
return -EINVAL;
@@ -817,7 +825,11 @@ static int tipc_nl_compat_link_reset_stats(struct tipc_nl_compat_cmd_doit *cmd,
if (!link)
return -EMSGSIZE;
- len = min_t(int, TLV_GET_DATA_LEN(msg->req), TIPC_MAX_LINK_NAME);
+ len = TLV_GET_DATA_LEN(msg->req);
+ if (len <= 0)
+ return -EINVAL;
+
+ len = min_t(int, len, TIPC_MAX_BEARER_NAME);
if (!string_is_valid(name, len))
return -EINVAL;
--
2.1.0
^ permalink raw reply related
* Re: KASAN: global-out-of-bounds Read in qmi_wwan_probe
From: Bjørn Mork @ 2019-06-24 16:26 UTC (permalink / raw)
To: Hillf Danton
Cc: Kristian Evensen, syzbot, andreyknvl, davem, linux-kernel,
linux-usb, netdev, syzkaller-bugs
In-Reply-To: <871rzj6sww.fsf@miraculix.mork.no>
Hillf Danton <hdanton@sina.com> writes:
> and wonder if the following works.
>
> - info = (void *)&id->driver_info;
> + info = (void *)id->driver_info;
Doh! Right you are. Thanks to both you and Andrey for quick and good
help.
We obviously have some bad code patterns here, since this apparently
worked for Kristian by pure luck.
Bjørn
^ permalink raw reply
* [PATCH bpf-next v8 3/9] libbpf: support sockopt hooks
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Make libbpf aware of new sockopt hooks so it can derive prog type
and hook point from the section names.
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 5 +++++
tools/lib/bpf/libbpf_probes.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 68f45a96769f..cacbd038ac79 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2646,6 +2646,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
case BPF_PROG_TYPE_PERF_EVENT:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
return false;
case BPF_PROG_TYPE_KPROBE:
default:
@@ -3604,6 +3605,10 @@ static const struct {
BPF_CGROUP_UDP6_RECVMSG),
BPF_EAPROG_SEC("cgroup/sysctl", BPF_PROG_TYPE_CGROUP_SYSCTL,
BPF_CGROUP_SYSCTL),
+ BPF_EAPROG_SEC("cgroup/getsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT,
+ BPF_CGROUP_GETSOCKOPT),
+ BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT,
+ BPF_CGROUP_SETSOCKOPT),
};
#undef BPF_PROG_SEC_IMPL
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 6635a31a7a16..ace1a0708d99 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -101,6 +101,7 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
case BPF_PROG_TYPE_SK_REUSEPORT:
case BPF_PROG_TYPE_FLOW_DISSECTOR:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
default:
break;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 7/9] selftests/bpf: add sockopt test that exercises BPF_F_ALLOW_MULTI
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
sockopt test that verifies chaining behavior.
v7:
* rework the test to verify cgroup getsockopt chaining
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 4 +-
.../selftests/bpf/progs/sockopt_multi.c | 53 ++++
.../selftests/bpf/test_sockopt_multi.c | 276 ++++++++++++++++++
4 files changed, 333 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/sockopt_multi.c
create mode 100644 tools/testing/selftests/bpf/test_sockopt_multi.c
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 8ac076c311d4..a2f7f79c7908 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -41,3 +41,4 @@ test_btf_dump
xdping
test_sockopt
test_sockopt_sk
+test_sockopt_multi
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1210b4e98d27..5615a5f13ce4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -26,7 +26,8 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk
+ test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
+ test_sockopt_multi
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -105,6 +106,7 @@ $(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
$(OUTPUT)/test_sockopt: cgroup_helpers.c
$(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
+$(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/progs/sockopt_multi.c b/tools/testing/selftests/bpf/progs/sockopt_multi.c
new file mode 100644
index 000000000000..1529fe3afe9f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sockopt_multi.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <netinet/in.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
+
+SEC("cgroup/getsockopt/child")
+int _getsockopt_child(struct bpf_sockopt *ctx)
+{
+ __u8 *optval_end = ctx->optval_end;
+ __u8 *optval = ctx->optval;
+
+ if (ctx->level != SOL_IP || ctx->optname != IP_TOS)
+ return 1;
+
+ if (optval + 1 > optval_end)
+ return 0; /* EPERM, bounds check */
+
+ if (optval[0] != 0x80)
+ return 0; /* EPERM, unexpected optval from the kernel */
+
+ ctx->retval = 0; /* Reset system call return value to zero */
+
+ optval[0] = 0x90;
+ ctx->optlen = 1;
+
+ return 1;
+}
+
+SEC("cgroup/getsockopt/parent")
+int _getsockopt_parent(struct bpf_sockopt *ctx)
+{
+ __u8 *optval_end = ctx->optval_end;
+ __u8 *optval = ctx->optval;
+
+ if (ctx->level != SOL_IP || ctx->optname != IP_TOS)
+ return 1;
+
+ if (optval + 1 > optval_end)
+ return 0; /* EPERM, bounds check */
+
+ if (optval[0] != 0x90)
+ return 0; /* EPERM, unexpected optval from the kernel */
+
+ ctx->retval = 0; /* Reset system call return value to zero */
+
+ optval[0] = 0xA0;
+ ctx->optlen = 1;
+
+ return 1;
+}
diff --git a/tools/testing/selftests/bpf/test_sockopt_multi.c b/tools/testing/selftests/bpf/test_sockopt_multi.c
new file mode 100644
index 000000000000..71acdc861a55
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sockopt_multi.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <error.h>
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include <linux/filter.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "bpf_util.h"
+#include "cgroup_helpers.h"
+
+static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
+{
+ enum bpf_attach_type attach_type;
+ enum bpf_prog_type prog_type;
+ struct bpf_program *prog;
+ int err;
+
+ err = libbpf_prog_type_by_name(title, &prog_type, &attach_type);
+ if (err) {
+ log_err("Failed to deduct types for %s BPF program", title);
+ return -1;
+ }
+
+ prog = bpf_object__find_program_by_title(obj, title);
+ if (!prog) {
+ log_err("Failed to find %s BPF program", title);
+ return -1;
+ }
+
+ err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
+ attach_type, BPF_F_ALLOW_MULTI);
+ if (err) {
+ log_err("Failed to attach %s BPF program", title);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int prog_detach(struct bpf_object *obj, int cgroup_fd, const char *title)
+{
+ enum bpf_attach_type attach_type;
+ enum bpf_prog_type prog_type;
+ struct bpf_program *prog;
+ int err;
+
+ err = libbpf_prog_type_by_name(title, &prog_type, &attach_type);
+ if (err)
+ return -1;
+
+ prog = bpf_object__find_program_by_title(obj, title);
+ if (!prog)
+ return -1;
+
+ err = bpf_prog_detach2(bpf_program__fd(prog), cgroup_fd,
+ attach_type);
+ if (err)
+ return -1;
+
+ return 0;
+}
+
+static int run_test(struct bpf_object *obj, int cg_parent, int cg_child,
+ int sock_fd)
+{
+ socklen_t optlen;
+ __u8 buf;
+ int err;
+
+ /* Set IP_TOS to the expected value (0x80). */
+
+ buf = 0x80;
+ err = setsockopt(sock_fd, SOL_IP, IP_TOS, &buf, 1);
+ if (err < 0) {
+ log_err("Failed to call setsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ if (buf != 0x80) {
+ log_err("Unexpected getsockopt 0x%x != 0x80 without BPF", buf);
+ err = -1;
+ goto detach;
+ }
+
+ /* Attach child program and make sure it returns new value:
+ * - kernel: -> 0x80
+ * - child: 0x80 -> 0x90
+ */
+
+ err = prog_attach(obj, cg_child, "cgroup/getsockopt/child");
+ if (err)
+ goto detach;
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ if (buf != 0x90) {
+ log_err("Unexpected getsockopt 0x%x != 0x90", buf);
+ err = -1;
+ goto detach;
+ }
+
+ /* Attach parent program and make sure it returns new value:
+ * - kernel: -> 0x80
+ * - child: 0x80 -> 0x90
+ * - parent: 0x90 -> 0xA0
+ */
+
+ err = prog_attach(obj, cg_parent, "cgroup/getsockopt/parent");
+ if (err)
+ goto detach;
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ if (buf != 0xA0) {
+ log_err("Unexpected getsockopt 0x%x != 0xA0", buf);
+ err = -1;
+ goto detach;
+ }
+
+ /* Setting unexpected initial sockopt should return EPERM:
+ * - kernel: -> 0x40
+ * - child: unexpected 0x40, EPERM
+ * - parent: unexpected 0x40, EPERM
+ */
+
+ buf = 0x40;
+ if (setsockopt(sock_fd, SOL_IP, IP_TOS, &buf, 1) < 0) {
+ log_err("Failed to call setsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (!err) {
+ log_err("Unexpected success from getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ /* Detach child program and make sure we still get EPERM:
+ * - kernel: -> 0x40
+ * - parent: unexpected 0x40, EPERM
+ */
+
+ err = prog_detach(obj, cg_child, "cgroup/getsockopt/child");
+ if (err) {
+ log_err("Failed to detach child program");
+ goto detach;
+ }
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (!err) {
+ log_err("Unexpected success from getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ /* Set initial value to the one the parent program expects:
+ * - kernel: -> 0x90
+ * - parent: 0x90 -> 0xA0
+ */
+
+ buf = 0x90;
+ err = setsockopt(sock_fd, SOL_IP, IP_TOS, &buf, 1);
+ if (err < 0) {
+ log_err("Failed to call setsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ buf = 0x00;
+ optlen = 1;
+ err = getsockopt(sock_fd, SOL_IP, IP_TOS, &buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt(IP_TOS)");
+ goto detach;
+ }
+
+ if (buf != 0xA0) {
+ log_err("Unexpected getsockopt 0x%x != 0xA0", buf);
+ err = -1;
+ goto detach;
+ }
+
+detach:
+ prog_detach(obj, cg_child, "cgroup/getsockopt/child");
+ prog_detach(obj, cg_parent, "cgroup/getsockopt/parent");
+
+ return err;
+}
+
+int main(int argc, char **argv)
+{
+ struct bpf_prog_load_attr attr = {
+ .file = "./sockopt_multi.o",
+ };
+ int cg_parent = -1, cg_child = -1;
+ struct bpf_object *obj = NULL;
+ int sock_fd = -1;
+ int err = -1;
+ int ignored;
+
+ if (setup_cgroup_environment()) {
+ log_err("Failed to setup cgroup environment\n");
+ goto out;
+ }
+
+ cg_parent = create_and_get_cgroup("/parent");
+ if (cg_parent < 0) {
+ log_err("Failed to create cgroup /parent\n");
+ goto out;
+ }
+
+ cg_child = create_and_get_cgroup("/parent/child");
+ if (cg_child < 0) {
+ log_err("Failed to create cgroup /parent/child\n");
+ goto out;
+ }
+
+ if (join_cgroup("/parent/child")) {
+ log_err("Failed to join cgroup /parent/child\n");
+ goto out;
+ }
+
+ err = bpf_prog_load_xattr(&attr, &obj, &ignored);
+ if (err) {
+ log_err("Failed to load BPF object");
+ goto out;
+ }
+
+ sock_fd = socket(AF_INET, SOCK_STREAM, 0);
+ if (sock_fd < 0) {
+ log_err("Failed to create socket");
+ goto out;
+ }
+
+ if (run_test(obj, cg_parent, cg_child, sock_fd))
+ err = -1;
+
+out:
+ close(sock_fd);
+ bpf_object__close(obj);
+ close(cg_child);
+ close(cg_parent);
+
+ printf("test_sockopt_multi: %s\n", err ? "FAILED" : "PASSED");
+ return err ? EXIT_FAILURE : EXIT_SUCCESS;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 9/9] bpftool: support cgroup sockopt
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf
Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau,
Jakub Kicinski
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Support sockopt prog type and cgroup hooks in the bpftool.
Cc: Martin Lau <kafai@fb.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 7 +++++--
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 3 ++-
tools/bpf/bpftool/bash-completion/bpftool | 9 ++++++---
tools/bpf/bpftool/cgroup.c | 5 ++++-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 3 ++-
6 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 324df15bf4cc..6ac98f08e9d0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -30,7 +30,8 @@ CGROUP COMMANDS
| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
| *ATTACH_TYPE* := { **ingress** | **egress** | **sock_create** | **sock_ops** | **device** |
| **bind4** | **bind6** | **post_bind4** | **post_bind6** | **connect4** | **connect6** |
-| **sendmsg4** | **sendmsg6** | **recvmsg4** | **recvmsg6** | **sysctl** }
+| **sendmsg4** | **sendmsg6** | **recvmsg4** | **recvmsg6** | **sysctl** |
+| **getsockopt** | **setsockopt** }
| *ATTACH_FLAGS* := { **multi** | **override** }
DESCRIPTION
@@ -91,7 +92,9 @@ DESCRIPTION
an unconnected udp4 socket (since 5.2);
**recvmsg6** call to recvfrom(2), recvmsg(2), recvmmsg(2) for
an unconnected udp6 socket (since 5.2);
- **sysctl** sysctl access (since 5.2).
+ **sysctl** sysctl access (since 5.2);
+ **getsockopt** call to getsockopt (since 5.3);
+ **setsockopt** call to setsockopt (since 5.3).
**bpftool cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
Detach *PROG* from the cgroup *CGROUP* and attach type
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 55dd06517a3b..1df637f85f94 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -40,7 +40,8 @@ PROG COMMANDS
| **lwt_seg6local** | **sockops** | **sk_skb** | **sk_msg** | **lirc_mode2** |
| **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6** |
-| **cgroup/recvmsg4** | **cgroup/recvmsg6** | **cgroup/sysctl**
+| **cgroup/recvmsg4** | **cgroup/recvmsg6** | **cgroup/sysctl** |
+| **cgroup/getsockopt** | **cgroup/setsockopt**
| }
| *ATTACH_TYPE* := {
| **msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index c98cb99867f6..08f35c787ab6 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -379,7 +379,8 @@ _bpftool()
cgroup/sendmsg4 cgroup/sendmsg6 \
cgroup/recvmsg4 cgroup/recvmsg6 \
cgroup/post_bind4 cgroup/post_bind6 \
- cgroup/sysctl" -- \
+ cgroup/sysctl cgroup/getsockopt \
+ cgroup/setsockopt" -- \
"$cur" ) )
return 0
;;
@@ -689,7 +690,8 @@ _bpftool()
attach|detach)
local ATTACH_TYPES='ingress egress sock_create sock_ops \
device bind4 bind6 post_bind4 post_bind6 connect4 \
- connect6 sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl'
+ connect6 sendmsg4 sendmsg6 recvmsg4 recvmsg6 sysctl \
+ getsockopt setsockopt'
local ATTACH_FLAGS='multi override'
local PROG_TYPE='id pinned tag'
case $prev in
@@ -699,7 +701,8 @@ _bpftool()
;;
ingress|egress|sock_create|sock_ops|device|bind4|bind6|\
post_bind4|post_bind6|connect4|connect6|sendmsg4|\
- sendmsg6|recvmsg4|recvmsg6|sysctl)
+ sendmsg6|recvmsg4|recvmsg6|sysctl|getsockopt|\
+ setsockopt)
COMPREPLY=( $( compgen -W "$PROG_TYPE" -- \
"$cur" ) )
return 0
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 1bb2a751107a..24937891bc48 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -26,7 +26,8 @@
" sock_ops | device | bind4 | bind6 |\n" \
" post_bind4 | post_bind6 | connect4 |\n" \
" connect6 | sendmsg4 | sendmsg6 |\n" \
- " recvmsg4 | recvmsg6 | sysctl }"
+ " recvmsg4 | recvmsg6 | sysctl |\n" \
+ " getsockopt | setsockopt }"
static const char * const attach_type_strings[] = {
[BPF_CGROUP_INET_INGRESS] = "ingress",
@@ -45,6 +46,8 @@ static const char * const attach_type_strings[] = {
[BPF_CGROUP_SYSCTL] = "sysctl",
[BPF_CGROUP_UDP4_RECVMSG] = "recvmsg4",
[BPF_CGROUP_UDP6_RECVMSG] = "recvmsg6",
+ [BPF_CGROUP_GETSOCKOPT] = "getsockopt",
+ [BPF_CGROUP_SETSOCKOPT] = "setsockopt",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index fddec15c454a..fb0ba77e6722 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -74,6 +74,7 @@ static const char * const prog_type_name[] = {
[BPF_PROG_TYPE_SK_REUSEPORT] = "sk_reuseport",
[BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
[BPF_PROG_TYPE_CGROUP_SYSCTL] = "cgroup_sysctl",
+ [BPF_PROG_TYPE_CGROUP_SOCKOPT] = "cgroup_sockopt",
};
extern const char * const map_type_name[];
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index f1a831f05010..9b0db5d14e31 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1071,7 +1071,8 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 | cgroup/recvmsg4 |\n"
- " cgroup/recvmsg6 }\n"
+ " cgroup/recvmsg6 | cgroup/getsockopt |\n"
+ " cgroup/setsockopt }\n"
" ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
" flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 8/9] bpf: add sockopt documentation
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Provide user documentation about sockopt prog type and cgroup hooks.
v7:
* add description for retval=0 and optlen=-1
v6:
* describe cgroup chaining, add example
v2:
* use return code 2 for kernel bypass
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
Documentation/bpf/index.rst | 1 +
Documentation/bpf/prog_cgroup_sockopt.rst | 82 +++++++++++++++++++++++
2 files changed, 83 insertions(+)
create mode 100644 Documentation/bpf/prog_cgroup_sockopt.rst
diff --git a/Documentation/bpf/index.rst b/Documentation/bpf/index.rst
index d3fe4cac0c90..801a6ed3f2e5 100644
--- a/Documentation/bpf/index.rst
+++ b/Documentation/bpf/index.rst
@@ -42,6 +42,7 @@ Program types
.. toctree::
:maxdepth: 1
+ prog_cgroup_sockopt
prog_cgroup_sysctl
prog_flow_dissector
diff --git a/Documentation/bpf/prog_cgroup_sockopt.rst b/Documentation/bpf/prog_cgroup_sockopt.rst
new file mode 100644
index 000000000000..24985740711a
--- /dev/null
+++ b/Documentation/bpf/prog_cgroup_sockopt.rst
@@ -0,0 +1,82 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================
+BPF_PROG_TYPE_CGROUP_SOCKOPT
+============================
+
+``BPF_PROG_TYPE_CGROUP_SOCKOPT`` program type can be attached to two
+cgroup hooks:
+
+* ``BPF_CGROUP_GETSOCKOPT`` - called every time process executes ``getsockopt``
+ system call.
+* ``BPF_CGROUP_SETSOCKOPT`` - called every time process executes ``setsockopt``
+ system call.
+
+The context (``struct bpf_sockopt``) has associated socket (``sk``) and
+all input arguments: ``level``, ``optname``, ``optval`` and ``optlen``.
+
+BPF_CGROUP_SETSOCKOPT
+=====================
+
+``BPF_CGROUP_SETSOCKOPT`` is triggered *before* the kernel handling of
+sockopt and it has mostly read-only context (it can modify only ``optlen``).
+This hook has access to cgroup and socket local storage.
+
+If BPF program sets ``optlen`` to -1, the control will be returned
+back to the userspace after all other BPF programs in the cgroup
+chain finish (i.e. kernel ``setsockopt`` handling will *not* be executed).
+
+Note, that the only acceptable value to set to ``optlen`` is -1. Any
+other value will trigger ``EFAULT``.
+
+Return Type
+-----------
+
+* ``0`` - reject the syscall, ``EPERM`` will be returned to the userspace.
+* ``1`` - success, continue with next BPF program in the cgroup chain.
+
+BPF_CGROUP_GETSOCKOPT
+=====================
+
+``BPF_CGROUP_GETSOCKOPT`` is triggered *after* the kernel handing of
+sockopt. The BPF hook can observe ``optval``, ``optlen`` and ``retval``
+if it's interested in whatever kernel has returned. BPF hook can override
+the values above, adjust ``optlen`` and reset ``retval`` to 0. If ``optlen``
+has been increased above initial ``setsockopt`` value (i.e. userspace
+buffer is too small), ``EFAULT`` is returned.
+
+Note, that the only acceptable value to set to ``retval`` is 0 and the
+original value that the kernel returned. Any other value will trigger
+``EFAULT``.
+
+Return Type
+-----------
+
+* ``0`` - reject the syscall, ``EPERM`` will be returned to the userspace.
+* ``1`` - success: copy ``optval`` and ``optlen`` to userspace, return
+ ``retval`` from the syscall (note that this can be overwritten by
+ the BPF program from the parent cgroup).
+
+Cgroup Inheritance
+==================
+
+Suppose, there is the following cgroup hierarchy where each cgroup
+has ``BPF_CGROUP_GETSOCKOPT`` attached at each level with
+``BPF_F_ALLOW_MULTI`` flag::
+
+ A (root, parent)
+ \
+ B (child)
+
+When the application calls ``getsockopt`` syscall from the cgroup B,
+the programs are executed from the bottom up: B, A. First program
+(B) sees the result of kernel's ``getsockopt``. It can optionally
+adjust ``optval``, ``optlen`` and reset ``retval`` to 0. After that
+control will be passed to the second (A) program which will see the
+same context as B including any potential modifications.
+
+Example
+=======
+
+See ``tools/testing/selftests/bpf/progs/sockopt_sk.c`` for an example
+of BPF program that handles socket options.
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 5/9] selftests/bpf: add sockopt test
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Add sockopt selftests:
* require proper expected_attach_type
* enforce context field read/write access
* test bpf_sockopt_handled handler
* test EPERM
* test limiting optlen from getsockopt
* test out-of-bounds access
v7:
* remove return 2; test retval=0 and optlen=-1
v3:
* use DW for optval{,_end} loads
v2:
* use return code 2 for kernel bypass
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/test_sockopt.c | 892 +++++++++++++++++++++
3 files changed, 895 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/test_sockopt.c
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 7470327edcfe..3fe92601223d 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -39,3 +39,4 @@ libbpf.so.*
test_hashmap
test_btf_dump
xdping
+test_sockopt
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f2dbe2043067..afd7b3d298d8 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -26,7 +26,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping
+ test_btf_dump test_cgroup_attach xdping test_sockopt
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -103,6 +103,7 @@ $(OUTPUT)/test_netcnt: cgroup_helpers.c
$(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
+$(OUTPUT)/test_sockopt: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/test_sockopt.c b/tools/testing/selftests/bpf/test_sockopt.c
new file mode 100644
index 000000000000..0c4196eecc45
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sockopt.c
@@ -0,0 +1,892 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include <linux/filter.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "bpf_util.h"
+#include "cgroup_helpers.h"
+
+#define CG_PATH "/sockopt"
+
+static char bpf_log_buf[4096];
+static bool verbose;
+
+enum sockopt_test_error {
+ OK = 0,
+ DENY_LOAD,
+ DENY_ATTACH,
+ EPERM_GETSOCKOPT,
+ EFAULT_GETSOCKOPT,
+ EPERM_SETSOCKOPT,
+ EFAULT_SETSOCKOPT,
+};
+
+static struct sockopt_test {
+ const char *descr;
+ const struct bpf_insn insns[64];
+ enum bpf_attach_type attach_type;
+ enum bpf_attach_type expected_attach_type;
+
+ int level;
+ int optname;
+
+ const char set_optval[64];
+ socklen_t set_optlen;
+
+ const char get_optval[64];
+ socklen_t get_optlen;
+ socklen_t get_optlen_ret;
+
+ enum sockopt_test_error error;
+} tests[] = {
+
+ /* ==================== getsockopt ==================== */
+
+ {
+ .descr = "getsockopt: no expected_attach_type",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = 0,
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: wrong expected_attach_type",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+ .error = DENY_ATTACH,
+ },
+ {
+ .descr = "getsockopt: bypass bpf hook",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .set_optval = { 1 << 3 },
+ .set_optlen = 1,
+
+ .get_optval = { 1 << 3 },
+ .get_optlen = 1,
+ },
+ {
+ .descr = "getsockopt: return EPERM from bpf hook",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .get_optlen = 1,
+ .error = EPERM_GETSOCKOPT,
+ },
+ {
+ .descr = "getsockopt: no optval bounds check, deny loading",
+ .insns = {
+ /* r6 = ctx->optval */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval)),
+
+ /* ctx->optval[0] = 0x80 */
+ BPF_MOV64_IMM(BPF_REG_0, 0x80),
+ BPF_STX_MEM(BPF_W, BPF_REG_6, BPF_REG_0, 0),
+
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: read ctx->level",
+ .insns = {
+ /* r6 = ctx->level */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, level)),
+
+ /* if (ctx->level == 123) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 123, 4),
+ /* 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)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .level = 123,
+
+ .get_optlen = 1,
+ },
+ {
+ .descr = "getsockopt: deny writing to ctx->level",
+ .insns = {
+ /* ctx->level = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, level)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: read ctx->optname",
+ .insns = {
+ /* r6 = ctx->optname */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optname)),
+
+ /* if (ctx->optname == 123) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 123, 4),
+ /* 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)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .optname = 123,
+
+ .get_optlen = 1,
+ },
+ {
+ .descr = "getsockopt: deny writing to ctx->optname",
+ .insns = {
+ /* ctx->optname = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optname)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: read ctx->optlen",
+ .insns = {
+ /* r6 = ctx->optlen */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optlen)),
+
+ /* if (ctx->optlen == 64) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 64, 4),
+ /* 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)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .get_optlen = 64,
+ },
+ {
+ .descr = "getsockopt: deny bigger ctx->optlen",
+ .insns = {
+ /* ctx->optlen = 65 */
+ BPF_MOV64_IMM(BPF_REG_0, 65),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+
+ /* 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)),
+
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .get_optlen = 64,
+
+ .error = EFAULT_GETSOCKOPT,
+ },
+ {
+ .descr = "getsockopt: deny arbitrary ctx->retval",
+ .insns = {
+ /* ctx->retval = 123 */
+ BPF_MOV64_IMM(BPF_REG_0, 123),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, retval)),
+
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .get_optlen = 64,
+
+ .error = EFAULT_GETSOCKOPT,
+ },
+ {
+ .descr = "getsockopt: support smaller ctx->optlen",
+ .insns = {
+ /* ctx->optlen = 32 */
+ BPF_MOV64_IMM(BPF_REG_0, 32),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ /* 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)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .get_optlen = 64,
+ .get_optlen_ret = 32,
+ },
+ {
+ .descr = "getsockopt: deny writing to ctx->optval",
+ .insns = {
+ /* ctx->optval = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optval)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: deny writing to ctx->optval_end",
+ .insns = {
+ /* ctx->optval_end = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optval_end)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "getsockopt: rewrite value",
+ .insns = {
+ /* 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, 0xF0),
+ /* } */
+
+ /* 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)),
+
+ /* return 1*/
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_GETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .get_optval = { 0xF0 },
+ .get_optlen = 1,
+ },
+
+ /* ==================== setsockopt ==================== */
+
+ {
+ .descr = "setsockopt: no expected_attach_type",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = 0,
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: wrong expected_attach_type",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_GETSOCKOPT,
+ .error = DENY_ATTACH,
+ },
+ {
+ .descr = "setsockopt: bypass bpf hook",
+ .insns = {
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .set_optval = { 1 << 3 },
+ .set_optlen = 1,
+
+ .get_optval = { 1 << 3 },
+ .get_optlen = 1,
+ },
+ {
+ .descr = "setsockopt: return EPERM from bpf hook",
+ .insns = {
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .set_optlen = 1,
+ .error = EPERM_SETSOCKOPT,
+ },
+ {
+ .descr = "setsockopt: no optval bounds check, deny loading",
+ .insns = {
+ /* r6 = ctx->optval */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval)),
+
+ /* r0 = ctx->optval[0] */
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_6, 0),
+
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: read ctx->level",
+ .insns = {
+ /* r6 = ctx->level */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, level)),
+
+ /* if (ctx->level == 123) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 123, 4),
+ /* ctx->optlen = -1 */
+ BPF_MOV64_IMM(BPF_REG_0, -1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .level = 123,
+
+ .set_optlen = 1,
+ },
+ {
+ .descr = "setsockopt: deny writing to ctx->level",
+ .insns = {
+ /* ctx->level = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, level)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: read ctx->optname",
+ .insns = {
+ /* r6 = ctx->optname */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optname)),
+
+ /* if (ctx->optname == 123) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 123, 4),
+ /* ctx->optlen = -1 */
+ BPF_MOV64_IMM(BPF_REG_0, -1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .optname = 123,
+
+ .set_optlen = 1,
+ },
+ {
+ .descr = "setsockopt: deny writing to ctx->optname",
+ .insns = {
+ /* ctx->optname = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optname)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: read ctx->optlen",
+ .insns = {
+ /* r6 = ctx->optlen */
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optlen)),
+
+ /* if (ctx->optlen == 64) { */
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 64, 4),
+ /* ctx->optlen = -1 */
+ BPF_MOV64_IMM(BPF_REG_0, -1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } else { */
+ /* return 0 */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .set_optlen = 64,
+ },
+ {
+ .descr = "setsockopt: ctx->optlen == -1 is ok",
+ .insns = {
+ /* ctx->optlen = -1 */
+ BPF_MOV64_IMM(BPF_REG_0, -1),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .set_optlen = 64,
+ },
+ {
+ .descr = "setsockopt: deny ctx->optlen != input optlen",
+ .insns = {
+ /* ctx->optlen = 32 */
+ BPF_MOV64_IMM(BPF_REG_0, 32),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optlen)),
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .set_optlen = 64,
+
+ .error = EFAULT_SETSOCKOPT,
+ },
+ {
+ .descr = "setsockopt: deny ctx->retval",
+ .insns = {
+ /* 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)),
+
+ /* return 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .get_optlen = 64,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: deny writing to ctx->optval",
+ .insns = {
+ /* ctx->optval = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optval)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: deny writing to ctx->optval_end",
+ .insns = {
+ /* ctx->optval_end = 1 */
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0,
+ offsetof(struct bpf_sockopt, optval_end)),
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .error = DENY_LOAD,
+ },
+ {
+ .descr = "setsockopt: allow IP_TOS <= 128",
+ .insns = {
+ /* r6 = ctx->optval */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval)),
+ /* r7 = ctx->optval + 1 */
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_6),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, 1),
+
+ /* r8 = ctx->optval_end */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval_end)),
+
+ /* if (ctx->optval + 1 <= ctx->optval_end) { */
+ BPF_JMP_REG(BPF_JGT, BPF_REG_7, BPF_REG_8, 4),
+
+ /* r9 = ctx->optval[0] */
+ BPF_LDX_MEM(BPF_B, BPF_REG_9, BPF_REG_6, 0),
+
+ /* if (ctx->optval[0] < 128) */
+ BPF_JMP_IMM(BPF_JGT, BPF_REG_9, 128, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } */
+
+ /* } else { */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .set_optval = { 0x80 },
+ .set_optlen = 1,
+ .get_optval = { 0x80 },
+ .get_optlen = 1,
+ },
+ {
+ .descr = "setsockopt: deny IP_TOS > 128",
+ .insns = {
+ /* r6 = ctx->optval */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval)),
+ /* r7 = ctx->optval + 1 */
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_6),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_7, 1),
+
+ /* r8 = ctx->optval_end */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_1,
+ offsetof(struct bpf_sockopt, optval_end)),
+
+ /* if (ctx->optval + 1 <= ctx->optval_end) { */
+ BPF_JMP_REG(BPF_JGT, BPF_REG_7, BPF_REG_8, 4),
+
+ /* r9 = ctx->optval[0] */
+ BPF_LDX_MEM(BPF_B, BPF_REG_9, BPF_REG_6, 0),
+
+ /* if (ctx->optval[0] < 128) */
+ BPF_JMP_IMM(BPF_JGT, BPF_REG_9, 128, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_JMP_A(1),
+ /* } */
+
+ /* } else { */
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ /* } */
+
+ BPF_EXIT_INSN(),
+ },
+ .attach_type = BPF_CGROUP_SETSOCKOPT,
+ .expected_attach_type = BPF_CGROUP_SETSOCKOPT,
+
+ .level = SOL_IP,
+ .optname = IP_TOS,
+
+ .set_optval = { 0x81 },
+ .set_optlen = 1,
+ .get_optval = { 0x00 },
+ .get_optlen = 1,
+
+ .error = EPERM_SETSOCKOPT,
+ },
+};
+
+static int load_prog(const struct bpf_insn *insns,
+ enum bpf_attach_type expected_attach_type)
+{
+ struct bpf_load_program_attr attr = {
+ .prog_type = BPF_PROG_TYPE_CGROUP_SOCKOPT,
+ .expected_attach_type = expected_attach_type,
+ .insns = insns,
+ .license = "GPL",
+ .log_level = 2,
+ };
+ int fd;
+
+ for (;
+ insns[attr.insns_cnt].code != (BPF_JMP | BPF_EXIT);
+ attr.insns_cnt++) {
+ }
+ attr.insns_cnt++;
+
+ fd = bpf_load_program_xattr(&attr, bpf_log_buf, sizeof(bpf_log_buf));
+ if (verbose && fd < 0)
+ fprintf(stderr, "%s\n", bpf_log_buf);
+
+ return fd;
+}
+
+static int run_test(int cgroup_fd, struct sockopt_test *test)
+{
+ int sock_fd, err, prog_fd;
+ void *optval = NULL;
+ int ret = 0;
+
+ prog_fd = load_prog(test->insns, test->expected_attach_type);
+ if (prog_fd < 0) {
+ if (test->error == DENY_LOAD)
+ return 0;
+
+ log_err("Failed to load BPF program");
+ return -1;
+ }
+
+ err = bpf_prog_attach(prog_fd, cgroup_fd, test->attach_type, 0);
+ if (err < 0) {
+ if (test->error == DENY_ATTACH)
+ goto close_prog_fd;
+
+ log_err("Failed to attach BPF program");
+ ret = -1;
+ goto close_prog_fd;
+ }
+
+ sock_fd = socket(AF_INET, SOCK_STREAM, 0);
+ if (sock_fd < 0) {
+ log_err("Failed to create AF_INET socket");
+ ret = -1;
+ goto detach_prog;
+ }
+
+ if (test->set_optlen) {
+ err = setsockopt(sock_fd, test->level, test->optname,
+ test->set_optval, test->set_optlen);
+ if (err) {
+ if (errno == EPERM && test->error == EPERM_SETSOCKOPT)
+ goto close_sock_fd;
+ if (errno == EFAULT && test->error == EFAULT_SETSOCKOPT)
+ goto free_optval;
+
+ log_err("Failed to call setsockopt");
+ ret = -1;
+ goto close_sock_fd;
+ }
+ }
+
+ if (test->get_optlen) {
+ optval = malloc(test->get_optlen);
+ socklen_t optlen = test->get_optlen;
+ socklen_t expected_get_optlen = test->get_optlen_ret ?:
+ test->get_optlen;
+
+ err = getsockopt(sock_fd, test->level, test->optname,
+ optval, &optlen);
+ if (err) {
+ if (errno == EPERM && test->error == EPERM_GETSOCKOPT)
+ goto free_optval;
+ if (errno == EFAULT && test->error == EFAULT_GETSOCKOPT)
+ goto free_optval;
+
+ log_err("Failed to call getsockopt");
+ ret = -1;
+ goto free_optval;
+ }
+
+ if (optlen != expected_get_optlen) {
+ errno = 0;
+ log_err("getsockopt returned unexpected optlen");
+ ret = -1;
+ goto free_optval;
+ }
+
+ if (memcmp(optval, test->get_optval, optlen) != 0) {
+ errno = 0;
+ log_err("getsockopt returned unexpected optval");
+ ret = -1;
+ goto free_optval;
+ }
+ }
+
+ ret = test->error != OK;
+
+free_optval:
+ free(optval);
+close_sock_fd:
+ close(sock_fd);
+detach_prog:
+ bpf_prog_detach2(prog_fd, cgroup_fd, test->attach_type);
+close_prog_fd:
+ close(prog_fd);
+ return ret;
+}
+
+int main(int args, char **argv)
+{
+ int err = EXIT_FAILURE, error_cnt = 0;
+ int cgroup_fd, i;
+
+ if (setup_cgroup_environment())
+ goto cleanup_obj;
+
+ cgroup_fd = create_and_get_cgroup(CG_PATH);
+ if (cgroup_fd < 0)
+ goto cleanup_cgroup_env;
+
+ if (join_cgroup(CG_PATH))
+ goto cleanup_cgroup;
+
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ int err = run_test(cgroup_fd, &tests[i]);
+
+ if (err)
+ error_cnt++;
+
+ printf("#%d %s: %s\n", i, err ? "FAIL" : "PASS",
+ tests[i].descr);
+ }
+
+ printf("Summary: %ld PASSED, %d FAILED\n",
+ ARRAY_SIZE(tests) - error_cnt, error_cnt);
+ err = error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
+
+cleanup_cgroup:
+ close(cgroup_fd);
+cleanup_cgroup_env:
+ cleanup_cgroup_environment();
+cleanup_obj:
+ return err;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 6/9] selftests/bpf: add sockopt test that exercises sk helpers
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
socktop test that introduces new SOL_CUSTOM sockopt level and
stores whatever users sets in sk storage. Whenever getsockopt
is called, the original value is retrieved.
v7:
* use retval=0 and optlen-1
v6:
* test 'ret=1' use-case as well (Alexei Starovoitov)
v4:
* don't call bpf_sk_fullsock helper
v3:
* drop (__u8 *)(long) casts for optval{,_end}
v2:
* new test
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 3 +-
.../testing/selftests/bpf/progs/sockopt_sk.c | 91 +++++++++
tools/testing/selftests/bpf/test_sockopt_sk.c | 185 ++++++++++++++++++
4 files changed, 279 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/sockopt_sk.c
create mode 100644 tools/testing/selftests/bpf/test_sockopt_sk.c
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 3fe92601223d..8ac076c311d4 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -40,3 +40,4 @@ test_hashmap
test_btf_dump
xdping
test_sockopt
+test_sockopt_sk
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index afd7b3d298d8..1210b4e98d27 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -26,7 +26,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
test_cgroup_storage test_select_reuseport test_section_names \
test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
- test_btf_dump test_cgroup_attach xdping test_sockopt
+ test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk
BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
TEST_GEN_FILES = $(BPF_OBJ_FILES)
@@ -104,6 +104,7 @@ $(OUTPUT)/test_sock_fields: cgroup_helpers.c
$(OUTPUT)/test_sysctl: cgroup_helpers.c
$(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
$(OUTPUT)/test_sockopt: cgroup_helpers.c
+$(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
new file mode 100644
index 000000000000..ad4c5db2bc29
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <netinet/in.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
+
+#define SOL_CUSTOM 0xdeadbeef
+
+struct sockopt_sk {
+ __u8 val;
+};
+
+struct bpf_map_def SEC("maps") socket_storage_map = {
+ .type = BPF_MAP_TYPE_SK_STORAGE,
+ .key_size = sizeof(int),
+ .value_size = sizeof(struct sockopt_sk),
+ .map_flags = BPF_F_NO_PREALLOC,
+};
+BPF_ANNOTATE_KV_PAIR(socket_storage_map, int, struct sockopt_sk);
+
+SEC("cgroup/getsockopt")
+int _getsockopt(struct bpf_sockopt *ctx)
+{
+ __u8 *optval_end = ctx->optval_end;
+ __u8 *optval = ctx->optval;
+ struct sockopt_sk *storage;
+
+ if (ctx->level == SOL_IP && ctx->optname == IP_TOS)
+ /* Not interested in SOL_IP:IP_TOS;
+ * let next BPF program in the cgroup chain or kernel
+ * handle it.
+ */
+ return 1;
+
+ if (ctx->level != SOL_CUSTOM)
+ return 0; /* EPERM, deny everything except custom level */
+
+ if (optval + 1 > optval_end)
+ return 0; /* EPERM, bounds check */
+
+ storage = bpf_sk_storage_get(&socket_storage_map, ctx->sk, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0; /* EPERM, couldn't get sk storage */
+
+ if (!ctx->retval)
+ return 0; /* EPERM, kernel should not have handled
+ * SOL_CUSTOM, something is wrong!
+ */
+ ctx->retval = 0; /* Reset system call return value to zero */
+
+ optval[0] = storage->val;
+ ctx->optlen = 1;
+
+ return 1;
+}
+
+SEC("cgroup/setsockopt")
+int _setsockopt(struct bpf_sockopt *ctx)
+{
+ __u8 *optval_end = ctx->optval_end;
+ __u8 *optval = ctx->optval;
+ struct sockopt_sk *storage;
+
+ if (ctx->level == SOL_IP && ctx->optname == IP_TOS)
+ /* Not interested in SOL_IP:IP_TOS;
+ * let next BPF program in the cgroup chain or kernel
+ * handle it.
+ */
+ return 1;
+
+ if (ctx->level != SOL_CUSTOM)
+ return 0; /* EPERM, deny everything except custom level */
+
+ if (optval + 1 > optval_end)
+ return 0; /* EPERM, bounds check */
+
+ storage = bpf_sk_storage_get(&socket_storage_map, ctx->sk, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0; /* EPERM, couldn't get sk storage */
+
+ storage->val = optval[0];
+ ctx->optlen = -1; /* BPF has consumed this option, don't call kernel
+ * setsockopt handler.
+ */
+
+ return 1;
+}
diff --git a/tools/testing/selftests/bpf/test_sockopt_sk.c b/tools/testing/selftests/bpf/test_sockopt_sk.c
new file mode 100644
index 000000000000..67558e2c5427
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sockopt_sk.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include <linux/filter.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "bpf_util.h"
+#include "cgroup_helpers.h"
+
+#define CG_PATH "/sockopt"
+
+#define SOL_CUSTOM 0xdeadbeef
+
+static int getsetsockopt(void)
+{
+ int fd, err;
+ char buf[4] = {};
+ socklen_t optlen;
+
+ fd = socket(AF_INET, SOCK_STREAM, 0);
+ if (fd < 0) {
+ log_err("Failed to create socket");
+ return -1;
+ }
+
+ /* IP_TOS - BPF bypass */
+
+ buf[0] = 0x08;
+ err = setsockopt(fd, SOL_IP, IP_TOS, buf, 1);
+ if (err) {
+ log_err("Failed to call setsockopt(IP_TOS)");
+ goto err;
+ }
+
+ buf[0] = 0x00;
+ optlen = 1;
+ err = getsockopt(fd, SOL_IP, IP_TOS, buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt(IP_TOS)");
+ goto err;
+ }
+
+ if (buf[0] != 0x08) {
+ log_err("Unexpected getsockopt(IP_TOS) buf[0] 0x%02x != 0x08",
+ buf[0]);
+ goto err;
+ }
+
+ /* IP_TTL - EPERM */
+
+ buf[0] = 1;
+ err = setsockopt(fd, SOL_IP, IP_TTL, buf, 1);
+ if (!err || errno != EPERM) {
+ log_err("Unexpected success from setsockopt(IP_TTL)");
+ goto err;
+ }
+
+ /* SOL_CUSTOM - handled by BPF */
+
+ buf[0] = 0x01;
+ err = setsockopt(fd, SOL_CUSTOM, 0, buf, 1);
+ if (err) {
+ log_err("Failed to call setsockopt");
+ goto err;
+ }
+
+ buf[0] = 0x00;
+ optlen = 4;
+ err = getsockopt(fd, SOL_CUSTOM, 0, buf, &optlen);
+ if (err) {
+ log_err("Failed to call getsockopt");
+ goto err;
+ }
+
+ if (optlen != 1) {
+ log_err("Unexpected optlen %d != 1", optlen);
+ goto err;
+ }
+ if (buf[0] != 0x01) {
+ log_err("Unexpected buf[0] 0x%02x != 0x01", buf[0]);
+ goto err;
+ }
+
+ close(fd);
+ return 0;
+err:
+ close(fd);
+ return -1;
+}
+
+static int prog_attach(struct bpf_object *obj, int cgroup_fd, const char *title)
+{
+ enum bpf_attach_type attach_type;
+ enum bpf_prog_type prog_type;
+ struct bpf_program *prog;
+ int err;
+
+ err = libbpf_prog_type_by_name(title, &prog_type, &attach_type);
+ if (err) {
+ log_err("Failed to deduct types for %s BPF program", title);
+ return -1;
+ }
+
+ prog = bpf_object__find_program_by_title(obj, title);
+ if (!prog) {
+ log_err("Failed to find %s BPF program", title);
+ return -1;
+ }
+
+ err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
+ attach_type, 0);
+ if (err) {
+ log_err("Failed to attach %s BPF program", title);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int run_test(int cgroup_fd)
+{
+ struct bpf_prog_load_attr attr = {
+ .file = "./sockopt_sk.o",
+ };
+ struct bpf_object *obj;
+ int ignored;
+ int err;
+
+ err = bpf_prog_load_xattr(&attr, &obj, &ignored);
+ if (err) {
+ log_err("Failed to load BPF object");
+ return -1;
+ }
+
+ err = prog_attach(obj, cgroup_fd, "cgroup/getsockopt");
+ if (err)
+ goto close_bpf_object;
+
+ err = prog_attach(obj, cgroup_fd, "cgroup/setsockopt");
+ if (err)
+ goto close_bpf_object;
+
+ err = getsetsockopt();
+
+close_bpf_object:
+ bpf_object__close(obj);
+ return err;
+}
+
+int main(int args, char **argv)
+{
+ int cgroup_fd;
+ int err = EXIT_SUCCESS;
+
+ if (setup_cgroup_environment())
+ goto cleanup_obj;
+
+ cgroup_fd = create_and_get_cgroup(CG_PATH);
+ if (cgroup_fd < 0)
+ goto cleanup_cgroup_env;
+
+ if (join_cgroup(CG_PATH))
+ goto cleanup_cgroup;
+
+ if (run_test(cgroup_fd))
+ err = EXIT_FAILURE;
+
+ printf("test_sockopt_sk: %s\n",
+ err == EXIT_SUCCESS ? "PASSED" : "FAILED");
+
+cleanup_cgroup:
+ close(cgroup_fd);
+cleanup_cgroup_env:
+ cleanup_cgroup_environment();
+cleanup_obj:
+ return err;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 4/9] selftests/bpf: test sockopt section name
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Add tests that make sure libbpf section detection works.
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_section_names.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_section_names.c b/tools/testing/selftests/bpf/test_section_names.c
index dee2f2eceb0f..29833aeaf0de 100644
--- a/tools/testing/selftests/bpf/test_section_names.c
+++ b/tools/testing/selftests/bpf/test_section_names.c
@@ -134,6 +134,16 @@ static struct sec_name_test tests[] = {
{0, BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL},
{0, BPF_CGROUP_SYSCTL},
},
+ {
+ "cgroup/getsockopt",
+ {0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT},
+ {0, BPF_CGROUP_GETSOCKOPT},
+ },
+ {
+ "cgroup/setsockopt",
+ {0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT},
+ {0, BPF_CGROUP_SETSOCKOPT},
+ },
};
static int test_prog_type_by_name(const struct sec_name_test *test)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH net-next 4/6] arm64: dts: fsl: ls1028a: Add Felix switch port DT node
From: Andrew Lunn @ 2019-06-24 16:24 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Alexandre Belloni, Claudiu Manoil, David S . Miller,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
UNGLinuxDriver@microchip.com, Allan Nielsen, Rob Herring,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190624152344.3bv46jjhhygo6zwl@lx-anielsen.microsemi.net>
On Mon, Jun 24, 2019 at 05:23:45PM +0200, Allan W. Nielsen wrote:
> Hi Andrew,
>
> The 06/24/2019 16:26, Andrew Lunn wrote:
> > > > Yeah, there are 2 ethernet controller ports (managed by the enetc driver)
> > > > connected inside the SoC via SGMII links to 2 of the switch ports, one of
> > > > these switch ports can be configured as CPU port (with follow-up patches).
> > > >
> > > > This configuration may look prettier on DSA, but the main restriction here
> > > > is that the entire functionality is provided by the ocelot driver which is a
> > > > switchdev driver. I don't think it would be a good idea to copy-paste code
> > > > from ocelot to a separate dsa driver.
> > > >
> > >
> > > We should probably make the ocelot driver a DSA driver then...
> > An important part of DSA is being able to direct frames out specific
> > ports when they ingress via the CPU port. Does the silicon support
> > this? At the moment, i think it is using polled IO.
>
> That is supported, it requires a bit of initial configuration of the Chip, but
> nothing big (I believe this configuration is part of Claudiu's change-set).
>
> But how do you envision this done?
>
> - Let the existing SwitchDev driver and the DSA driver use a set of common
> functions.
> - Convert the existing Ocelot driver from SwitchDev to DSA
> - Fork (copy) the existing driver of Ocelot, and modify it as needed for the
> Felix driver
>
> My guess is the first one, but I would like to understand what you have in mind.
I don't know the various architectures the switch is used in. But it
does seem like a core library, and then a switchdev wrapper for Ocelot
and a DSA wrapper for Felix would make sense.
Andrew
^ permalink raw reply
* [PATCH bpf-next v8 2/9] bpf: sync bpf.h to tools/
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Export new prog type and hook points to the libbpf.
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/include/uapi/linux/bpf.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b077507efa3f..a396b516a2b2 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -170,6 +170,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_FLOW_DISSECTOR,
BPF_PROG_TYPE_CGROUP_SYSCTL,
BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
+ BPF_PROG_TYPE_CGROUP_SOCKOPT,
};
enum bpf_attach_type {
@@ -194,6 +195,8 @@ enum bpf_attach_type {
BPF_CGROUP_SYSCTL,
BPF_CGROUP_UDP4_RECVMSG,
BPF_CGROUP_UDP6_RECVMSG,
+ BPF_CGROUP_GETSOCKOPT,
+ BPF_CGROUP_SETSOCKOPT,
__MAX_BPF_ATTACH_TYPE
};
@@ -3541,4 +3544,15 @@ struct bpf_sysctl {
*/
};
+struct bpf_sockopt {
+ __bpf_md_ptr(struct bpf_sock *, sk);
+ __bpf_md_ptr(void *, optval);
+ __bpf_md_ptr(void *, optval_end);
+
+ __s32 level;
+ __s32 optname;
+ __s32 optlen;
+ __s32 retval;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 1/9] bpf: implement getsockopt and setsockopt hooks
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin Lau
In-Reply-To: <20190624162429.16367-1-sdf@google.com>
Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.
The buffer memory is pre-allocated (because I don't think there is
a precedent for working with __user memory from bpf). This might be
slow to do for each {s,g}etsockopt call, that's why I've added
__cgroup_bpf_prog_array_is_empty that exits early if there is nothing
attached to a cgroup. Note, however, that there is a race between
__cgroup_bpf_prog_array_is_empty and BPF_PROG_RUN_ARRAY where cgroup
program layout might have changed; this should not be a problem
because in general there is a race between multiple calls to
{s,g}etsocktop and user adding/removing bpf progs from a cgroup.
The return code of the BPF program is handled as follows:
* 0: EPERM
* 1: success, continue with next BPF program in the cgroup chain
v8:
* use s32 for optlen (Andrii Nakryiko)
v7:
* return only 0 or 1 (Alexei Starovoitov)
* always run all progs (Alexei Starovoitov)
* use optval=0 as kernel bypass in setsockopt (Alexei Starovoitov)
(decided to use optval=-1 instead, optval=0 might be a valid input)
* call getsockopt hook after kernel handlers (Alexei Starovoitov)
v6:
* rework cgroup chaining; stop as soon as bpf program returns
0 or 2; see patch with the documentation for the details
* drop Andrii's and Martin's Acked-by (not sure they are comfortable
with the new state of things)
v5:
* skip copy_to_user() and put_user() when ret == 0 (Martin Lau)
v4:
* don't export bpf_sk_fullsock helper (Martin Lau)
* size != sizeof(__u64) for uapi pointers (Martin Lau)
* offsetof instead of bpf_ctx_range when checking ctx access (Martin Lau)
v3:
* typos in BPF_PROG_CGROUP_SOCKOPT_RUN_ARRAY comments (Andrii Nakryiko)
* reverse christmas tree in BPF_PROG_CGROUP_SOCKOPT_RUN_ARRAY (Andrii
Nakryiko)
* use __bpf_md_ptr instead of __u32 for optval{,_end} (Martin Lau)
* use BPF_FIELD_SIZEOF() for consistency (Martin Lau)
* new CG_SOCKOPT_ACCESS macro to wrap repeated parts
v2:
* moved bpf_sockopt_kern fields around to remove a hole (Martin Lau)
* aligned bpf_sockopt_kern->buf to 8 bytes (Martin Lau)
* bpf_prog_array_is_empty instead of bpf_prog_array_length (Martin Lau)
* added [0,2] return code check to verifier (Martin Lau)
* dropped unused buf[64] from the stack (Martin Lau)
* use PTR_TO_SOCKET for bpf_sockopt->sk (Martin Lau)
* dropped bpf_target_off from ctx rewrites (Martin Lau)
* use return code for kernel bypass (Martin Lau & Andrii Nakryiko)
Cc: Martin Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup.h | 43 +++++
include/linux/bpf.h | 2 +
include/linux/bpf_types.h | 1 +
include/linux/filter.h | 14 ++
include/uapi/linux/bpf.h | 14 ++
kernel/bpf/cgroup.c | 317 +++++++++++++++++++++++++++++++++++++
kernel/bpf/core.c | 9 ++
kernel/bpf/syscall.c | 19 +++
kernel/bpf/verifier.c | 13 ++
net/core/filter.c | 2 +-
net/socket.c | 16 ++
11 files changed, 449 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index bd79ae32909a..709d7c3ffdc3 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -124,6 +124,14 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
loff_t *ppos, void **new_buf,
enum bpf_attach_type type);
+int __cgroup_bpf_run_filter_setsockopt(struct sock *sock, int level,
+ int optname, char __user *optval,
+ int optlen);
+int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
+ int optname, char __user *optval,
+ int __user *optlen, int max_optlen,
+ int retval);
+
static inline enum bpf_cgroup_storage_type cgroup_storage_type(
struct bpf_map *map)
{
@@ -286,6 +294,36 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
__ret; \
})
+#define BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock, level, optname, optval, optlen) \
+({ \
+ int __ret = 0; \
+ if (cgroup_bpf_enabled) \
+ __ret = __cgroup_bpf_run_filter_setsockopt(sock, level, \
+ optname, optval, \
+ optlen); \
+ __ret; \
+})
+
+#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen) \
+({ \
+ int __ret = 0; \
+ if (cgroup_bpf_enabled) \
+ get_user(__ret, optlen); \
+ __ret; \
+})
+
+#define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, optlen, \
+ max_optlen, retval) \
+({ \
+ int __ret = retval; \
+ if (cgroup_bpf_enabled) \
+ __ret = __cgroup_bpf_run_filter_getsockopt(sock, level, \
+ optname, optval, \
+ optlen, max_optlen, \
+ retval); \
+ __ret; \
+})
+
int cgroup_bpf_prog_attach(const union bpf_attr *attr,
enum bpf_prog_type ptype, struct bpf_prog *prog);
int cgroup_bpf_prog_detach(const union bpf_attr *attr,
@@ -357,6 +395,11 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
#define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops) ({ 0; })
#define BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type,major,minor,access) ({ 0; })
#define BPF_CGROUP_RUN_PROG_SYSCTL(head,table,write,buf,count,pos,nbuf) ({ 0; })
+#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, \
+ optlen, max_optlen, retval) ({ retval; })
+#define BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock, level, optname, optval, \
+ optlen) ({ 0; })
#define for_each_cgroup_storage_type(stype) for (; false; )
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a62e7889b0b6..18f4cc2c6acd 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -518,6 +518,7 @@ struct bpf_prog_array {
struct bpf_prog_array *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
void bpf_prog_array_free(struct bpf_prog_array *progs);
int bpf_prog_array_length(struct bpf_prog_array *progs);
+bool bpf_prog_array_is_empty(struct bpf_prog_array *array);
int bpf_prog_array_copy_to_user(struct bpf_prog_array *progs,
__u32 __user *prog_ids, u32 cnt);
@@ -1051,6 +1052,7 @@ extern const struct bpf_func_proto bpf_spin_unlock_proto;
extern const struct bpf_func_proto bpf_get_local_storage_proto;
extern const struct bpf_func_proto bpf_strtol_proto;
extern const struct bpf_func_proto bpf_strtoul_proto;
+extern const struct bpf_func_proto bpf_tcp_sock_proto;
/* Shared helpers among cBPF and eBPF. */
void bpf_user_rnd_init_once(void);
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 5a9975678d6f..eec5aeeeaf92 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -30,6 +30,7 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, raw_tracepoint_writable)
#ifdef CONFIG_CGROUP_BPF
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SYSCTL, cg_sysctl)
+BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCKOPT, cg_sockopt)
#endif
#ifdef CONFIG_BPF_LIRC_MODE2
BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 43b45d6db36d..98f8a1210117 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1199,4 +1199,18 @@ struct bpf_sysctl_kern {
u64 tmp_reg;
};
+struct bpf_sockopt_kern {
+ struct sock *sk;
+ u8 *optval;
+ u8 *optval_end;
+ s32 level;
+ s32 optname;
+ s32 optlen;
+ s32 retval;
+
+ /* Small on-stack optval buffer to avoid small allocations.
+ */
+ u8 buf[64] __aligned(8);
+};
+
#endif /* __LINUX_FILTER_H__ */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b077507efa3f..a396b516a2b2 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -170,6 +170,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_FLOW_DISSECTOR,
BPF_PROG_TYPE_CGROUP_SYSCTL,
BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
+ BPF_PROG_TYPE_CGROUP_SOCKOPT,
};
enum bpf_attach_type {
@@ -194,6 +195,8 @@ enum bpf_attach_type {
BPF_CGROUP_SYSCTL,
BPF_CGROUP_UDP4_RECVMSG,
BPF_CGROUP_UDP6_RECVMSG,
+ BPF_CGROUP_GETSOCKOPT,
+ BPF_CGROUP_SETSOCKOPT,
__MAX_BPF_ATTACH_TYPE
};
@@ -3541,4 +3544,15 @@ struct bpf_sysctl {
*/
};
+struct bpf_sockopt {
+ __bpf_md_ptr(struct bpf_sock *, sk);
+ __bpf_md_ptr(void *, optval);
+ __bpf_md_ptr(void *, optval_end);
+
+ __s32 level;
+ __s32 optname;
+ __s32 optlen;
+ __s32 retval;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index c225c42e114a..255153f2d8e9 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -15,6 +15,7 @@
#include <linux/bpf.h>
#include <linux/bpf-cgroup.h>
#include <net/sock.h>
+#include <net/bpf_sk_storage.h>
DEFINE_STATIC_KEY_FALSE(cgroup_bpf_enabled_key);
EXPORT_SYMBOL(cgroup_bpf_enabled_key);
@@ -921,6 +922,185 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
}
EXPORT_SYMBOL(__cgroup_bpf_run_filter_sysctl);
+static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
+ enum bpf_attach_type attach_type)
+{
+ struct bpf_prog_array *prog_array;
+ bool empty;
+
+ rcu_read_lock();
+ prog_array = rcu_dereference(cgrp->bpf.effective[attach_type]);
+ empty = bpf_prog_array_is_empty(prog_array);
+ rcu_read_unlock();
+
+ return empty;
+}
+
+static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen)
+{
+ if (unlikely(max_optlen > PAGE_SIZE) || max_optlen < 0)
+ return -EINVAL;
+
+ if (likely(max_optlen <= sizeof(ctx->buf))) {
+ ctx->optval = ctx->buf;
+ } else {
+ ctx->optval = kzalloc(max_optlen, GFP_USER);
+ if (!ctx->optval)
+ return -ENOMEM;
+ }
+
+ ctx->optval_end = ctx->optval + max_optlen;
+ ctx->optlen = max_optlen;
+
+ return 0;
+}
+
+static void sockopt_free_buf(struct bpf_sockopt_kern *ctx)
+{
+ if (unlikely(ctx->optval != ctx->buf))
+ kfree(ctx->optval);
+}
+
+int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int level,
+ int optname, char __user *optval,
+ int optlen)
+{
+ struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
+ struct bpf_sockopt_kern ctx = {
+ .sk = sk,
+ .level = level,
+ .optname = optname,
+ };
+ int ret;
+
+ /* Opportunistic check to see whether we have any BPF program
+ * attached to the hook so we don't waste time allocating
+ * memory and locking the socket.
+ */
+ if (!cgroup_bpf_enabled ||
+ __cgroup_bpf_prog_array_is_empty(cgrp, BPF_CGROUP_SETSOCKOPT))
+ return 0;
+
+ ret = sockopt_alloc_buf(&ctx, optlen);
+ if (ret)
+ return ret;
+
+ if (copy_from_user(ctx.optval, optval, optlen) != 0) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ lock_sock(sk);
+ ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[BPF_CGROUP_SETSOCKOPT],
+ &ctx, BPF_PROG_RUN);
+ release_sock(sk);
+
+ if (!ret) {
+ ret = -EPERM;
+ goto out;
+ }
+
+ if (ctx.optlen == -1)
+ /* optlen set to -1, bypass kernel */
+ ret = 1;
+ else if (ctx.optlen == optlen)
+ /* optlen not changed, run kernel handler */
+ ret = 0;
+ else
+ /* any other value is rejected */
+ ret = -EFAULT;
+
+out:
+ sockopt_free_buf(&ctx);
+ return ret;
+}
+EXPORT_SYMBOL(__cgroup_bpf_run_filter_setsockopt);
+
+int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
+ int optname, char __user *optval,
+ int __user *optlen, int max_optlen,
+ int retval)
+{
+ struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
+ struct bpf_sockopt_kern ctx = {
+ .sk = sk,
+ .level = level,
+ .optname = optname,
+ .retval = retval,
+ };
+ int ret;
+
+ /* Opportunistic check to see whether we have any BPF program
+ * attached to the hook so we don't waste time allocating
+ * memory and locking the socket.
+ */
+ if (!cgroup_bpf_enabled ||
+ __cgroup_bpf_prog_array_is_empty(cgrp, BPF_CGROUP_GETSOCKOPT))
+ return retval;
+
+ ret = sockopt_alloc_buf(&ctx, max_optlen);
+ if (ret)
+ return ret;
+
+ if (!retval) {
+ /* If kernel getsockopt finished successfully,
+ * copy whatever was returned to the user back
+ * into our temporary buffer. Set optlen to the
+ * one that kernel returned as well to let
+ * BPF programs inspect the value.
+ */
+
+ if (get_user(ctx.optlen, optlen)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (ctx.optlen > max_optlen)
+ ctx.optlen = max_optlen;
+
+ if (copy_from_user(ctx.optval, optval, ctx.optlen) != 0) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+
+ lock_sock(sk);
+ ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[BPF_CGROUP_GETSOCKOPT],
+ &ctx, BPF_PROG_RUN);
+ release_sock(sk);
+
+ if (!ret) {
+ ret = -EPERM;
+ goto out;
+ }
+
+ if (ctx.optlen > max_optlen) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ /* BPF programs only allowed to set retval to 0, not some
+ * arbitrary value.
+ */
+ if (ctx.retval != 0 && ctx.retval != retval) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (copy_to_user(optval, ctx.optval, ctx.optlen) ||
+ put_user(ctx.optlen, optlen)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ ret = ctx.retval;
+
+out:
+ sockopt_free_buf(&ctx);
+ return ret;
+}
+EXPORT_SYMBOL(__cgroup_bpf_run_filter_getsockopt);
+
static ssize_t sysctl_cpy_dir(const struct ctl_dir *dir, char **bufp,
size_t *lenp)
{
@@ -1181,3 +1361,140 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = {
const struct bpf_prog_ops cg_sysctl_prog_ops = {
};
+
+static const struct bpf_func_proto *
+cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_sk_storage_get:
+ return &bpf_sk_storage_get_proto;
+ case BPF_FUNC_sk_storage_delete:
+ return &bpf_sk_storage_delete_proto;
+#ifdef CONFIG_INET
+ case BPF_FUNC_tcp_sock:
+ return &bpf_tcp_sock_proto;
+#endif
+ default:
+ return cgroup_base_func_proto(func_id, prog);
+ }
+}
+
+static bool cg_sockopt_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ const int size_default = sizeof(__u32);
+
+ if (off < 0 || off >= sizeof(struct bpf_sockopt))
+ return false;
+
+ if (off % size != 0)
+ return false;
+
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case offsetof(struct bpf_sockopt, retval):
+ if (size != size_default)
+ return false;
+ return prog->expected_attach_type ==
+ BPF_CGROUP_GETSOCKOPT;
+ case offsetof(struct bpf_sockopt, optlen):
+ return size == size_default;
+ default:
+ return false;
+ }
+ }
+
+ switch (off) {
+ case offsetof(struct bpf_sockopt, sk):
+ if (size != sizeof(__u64))
+ return false;
+ info->reg_type = PTR_TO_SOCKET;
+ break;
+ case offsetof(struct bpf_sockopt, optval):
+ if (size != sizeof(__u64))
+ return false;
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case offsetof(struct bpf_sockopt, optval_end):
+ if (size != sizeof(__u64))
+ return false;
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ case offsetof(struct bpf_sockopt, retval):
+ if (size != size_default)
+ return false;
+ return prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT;
+ default:
+ if (size != size_default)
+ return false;
+ break;
+ }
+ return true;
+}
+
+#define CG_SOCKOPT_ACCESS_FIELD(T, F) \
+ T(BPF_FIELD_SIZEOF(struct bpf_sockopt_kern, F), \
+ si->dst_reg, si->src_reg, \
+ offsetof(struct bpf_sockopt_kern, F))
+
+static u32 cg_sockopt_convert_ctx_access(enum bpf_access_type type,
+ const struct bpf_insn *si,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog,
+ u32 *target_size)
+{
+ struct bpf_insn *insn = insn_buf;
+
+ switch (si->off) {
+ case offsetof(struct bpf_sockopt, sk):
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, sk);
+ break;
+ case offsetof(struct bpf_sockopt, level):
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, level);
+ break;
+ case offsetof(struct bpf_sockopt, optname):
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, optname);
+ break;
+ case offsetof(struct bpf_sockopt, optlen):
+ if (type == BPF_WRITE)
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_STX_MEM, optlen);
+ else
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, optlen);
+ break;
+ case offsetof(struct bpf_sockopt, retval):
+ if (type == BPF_WRITE)
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_STX_MEM, retval);
+ else
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, retval);
+ break;
+ case offsetof(struct bpf_sockopt, optval):
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, optval);
+ break;
+ case offsetof(struct bpf_sockopt, optval_end):
+ *insn++ = CG_SOCKOPT_ACCESS_FIELD(BPF_LDX_MEM, optval_end);
+ break;
+ }
+
+ return insn - insn_buf;
+}
+
+static int cg_sockopt_get_prologue(struct bpf_insn *insn_buf,
+ bool direct_write,
+ const struct bpf_prog *prog)
+{
+ /* Nothing to do for sockopt argument. The data is kzalloc'ated.
+ */
+ return 0;
+}
+
+const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
+ .get_func_proto = cg_sockopt_func_proto,
+ .is_valid_access = cg_sockopt_is_valid_access,
+ .convert_ctx_access = cg_sockopt_convert_ctx_access,
+ .gen_prologue = cg_sockopt_get_prologue,
+};
+
+const struct bpf_prog_ops cg_sockopt_prog_ops = {
+};
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ad3be85f1411..accc3e2e4098 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1809,6 +1809,15 @@ int bpf_prog_array_length(struct bpf_prog_array *array)
return cnt;
}
+bool bpf_prog_array_is_empty(struct bpf_prog_array *array)
+{
+ struct bpf_prog_array_item *item;
+
+ for (item = array->items; item->prog; item++)
+ if (item->prog != &dummy_bpf_prog.prog)
+ return false;
+ return true;
+}
static bool bpf_prog_array_copy_core(struct bpf_prog_array *array,
u32 *prog_ids,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7713cf39795a..b0f545e07425 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1590,6 +1590,14 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
default:
return -EINVAL;
}
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
+ switch (expected_attach_type) {
+ case BPF_CGROUP_SETSOCKOPT:
+ case BPF_CGROUP_GETSOCKOPT:
+ return 0;
+ default:
+ return -EINVAL;
+ }
default:
return 0;
}
@@ -1840,6 +1848,7 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
switch (prog->type) {
case BPF_PROG_TYPE_CGROUP_SOCK:
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
case BPF_PROG_TYPE_CGROUP_SKB:
return prog->enforce_expected_attach_type &&
@@ -1912,6 +1921,10 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_CGROUP_SYSCTL:
ptype = BPF_PROG_TYPE_CGROUP_SYSCTL;
break;
+ case BPF_CGROUP_GETSOCKOPT:
+ case BPF_CGROUP_SETSOCKOPT:
+ ptype = BPF_PROG_TYPE_CGROUP_SOCKOPT;
+ break;
default:
return -EINVAL;
}
@@ -1995,6 +2008,10 @@ static int bpf_prog_detach(const union bpf_attr *attr)
case BPF_CGROUP_SYSCTL:
ptype = BPF_PROG_TYPE_CGROUP_SYSCTL;
break;
+ case BPF_CGROUP_GETSOCKOPT:
+ case BPF_CGROUP_SETSOCKOPT:
+ ptype = BPF_PROG_TYPE_CGROUP_SOCKOPT;
+ break;
default:
return -EINVAL;
}
@@ -2031,6 +2048,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
case BPF_CGROUP_SOCK_OPS:
case BPF_CGROUP_DEVICE:
case BPF_CGROUP_SYSCTL:
+ case BPF_CGROUP_GETSOCKOPT:
+ case BPF_CGROUP_SETSOCKOPT:
break;
case BPF_LIRC_MODE2:
return lirc_prog_query(attr, uattr);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e079b2298f8..08231e0449f8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2215,6 +2215,18 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
env->seen_direct_write = true;
return true;
+
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
+ if (t == BPF_WRITE) {
+ if (env->prog->expected_attach_type ==
+ BPF_CGROUP_GETSOCKOPT) {
+ env->seen_direct_write = true;
+ return true;
+ }
+ return false;
+ }
+ return true;
+
default:
return false;
}
@@ -6066,6 +6078,7 @@ static int check_return_code(struct bpf_verifier_env *env)
case BPF_PROG_TYPE_SOCK_OPS:
case BPF_PROG_TYPE_CGROUP_DEVICE:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
+ case BPF_PROG_TYPE_CGROUP_SOCKOPT:
break;
default:
return 0;
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..dc8534be12fc 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5651,7 +5651,7 @@ BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
return (unsigned long)NULL;
}
-static const struct bpf_func_proto bpf_tcp_sock_proto = {
+const struct bpf_func_proto bpf_tcp_sock_proto = {
.func = bpf_tcp_sock,
.gpl_only = false,
.ret_type = RET_PTR_TO_TCP_SOCK_OR_NULL,
diff --git a/net/socket.c b/net/socket.c
index 963df5dbdd54..18d7ce2d8b37 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2063,6 +2063,15 @@ static int __sys_setsockopt(int fd, int level, int optname,
if (err)
goto out_put;
+ err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, level, optname,
+ optval, optlen);
+ if (err < 0) {
+ goto out_put;
+ } else if (err > 0) {
+ err = 0;
+ goto out_put;
+ }
+
if (level == SOL_SOCKET)
err =
sock_setsockopt(sock, level, optname, optval,
@@ -2093,6 +2102,7 @@ static int __sys_getsockopt(int fd, int level, int optname,
{
int err, fput_needed;
struct socket *sock;
+ int max_optlen;
sock = sockfd_lookup_light(fd, &err, &fput_needed);
if (sock != NULL) {
@@ -2100,6 +2110,8 @@ static int __sys_getsockopt(int fd, int level, int optname,
if (err)
goto out_put;
+ max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
+
if (level == SOL_SOCKET)
err =
sock_getsockopt(sock, level, optname, optval,
@@ -2108,6 +2120,10 @@ static int __sys_getsockopt(int fd, int level, int optname,
err =
sock->ops->getsockopt(sock, level, optname, optval,
optlen);
+
+ err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level, optname,
+ optval, optlen,
+ max_optlen, err);
out_put:
fput_light(sock->file, fput_needed);
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v8 0/9] bpf: getsockopt and setsockopt hooks
From: Stanislav Fomichev @ 2019-06-24 16:24 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev
This series implements two new per-cgroup hooks: getsockopt and
setsockopt along with a new sockopt program type. The idea is pretty
similar to recently introduced cgroup sysctl hooks, but
implementation is simpler (no need to convert to/from strings).
What this can be applied to:
* move business logic of what tos/priority/etc can be set by
containers (either pass or reject)
* handle existing options (or introduce new ones) differently by
propagating some information in cgroup/socket local storage
Compared to a simple syscall/{g,s}etsockopt tracepoint, those
hooks are context aware. Meaning, they can access underlying socket
and use cgroup and socket local storage.
Stanislav Fomichev (9):
bpf: implement getsockopt and setsockopt hooks
bpf: sync bpf.h to tools/
libbpf: support sockopt hooks
selftests/bpf: test sockopt section name
selftests/bpf: add sockopt test
selftests/bpf: add sockopt test that exercises sk helpers
selftests/bpf: add sockopt test that exercises BPF_F_ALLOW_MULTI
bpf: add sockopt documentation
bpftool: support cgroup sockopt
Documentation/bpf/index.rst | 1 +
Documentation/bpf/prog_cgroup_sockopt.rst | 82 ++
include/linux/bpf-cgroup.h | 43 +
include/linux/bpf.h | 2 +
include/linux/bpf_types.h | 1 +
include/linux/filter.h | 14 +
include/uapi/linux/bpf.h | 14 +
kernel/bpf/cgroup.c | 317 +++++++
kernel/bpf/core.c | 9 +
kernel/bpf/syscall.c | 19 +
kernel/bpf/verifier.c | 13 +
net/core/filter.c | 2 +-
net/socket.c | 16 +
.../bpftool/Documentation/bpftool-cgroup.rst | 7 +-
.../bpftool/Documentation/bpftool-prog.rst | 3 +-
tools/bpf/bpftool/bash-completion/bpftool | 9 +-
tools/bpf/bpftool/cgroup.c | 5 +-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 3 +-
tools/include/uapi/linux/bpf.h | 14 +
tools/lib/bpf/libbpf.c | 5 +
tools/lib/bpf/libbpf_probes.c | 1 +
tools/testing/selftests/bpf/.gitignore | 3 +
tools/testing/selftests/bpf/Makefile | 6 +-
.../selftests/bpf/progs/sockopt_multi.c | 53 ++
.../testing/selftests/bpf/progs/sockopt_sk.c | 91 ++
.../selftests/bpf/test_section_names.c | 10 +
tools/testing/selftests/bpf/test_sockopt.c | 892 ++++++++++++++++++
.../selftests/bpf/test_sockopt_multi.c | 276 ++++++
tools/testing/selftests/bpf/test_sockopt_sk.c | 185 ++++
30 files changed, 2087 insertions(+), 10 deletions(-)
create mode 100644 Documentation/bpf/prog_cgroup_sockopt.rst
create mode 100644 tools/testing/selftests/bpf/progs/sockopt_multi.c
create mode 100644 tools/testing/selftests/bpf/progs/sockopt_sk.c
create mode 100644 tools/testing/selftests/bpf/test_sockopt.c
create mode 100644 tools/testing/selftests/bpf/test_sockopt_multi.c
create mode 100644 tools/testing/selftests/bpf/test_sockopt_sk.c
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH net] sctp: change to hold sk after auth shkey is created successfully
From: Xin Long @ 2019-06-24 16:21 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, syzkaller-bugs
Now in sctp_endpoint_init(), it holds the sk then creates auth
shkey. But when the creation fails, it doesn't release the sk,
which causes a sk defcnf leak,
Here to fix it by only holding the sk when auth shkey is created
successfully.
Fixes: a29a5bd4f5c3 ("[SCTP]: Implement SCTP-AUTH initializations.")
Reported-by: syzbot+afabda3890cc2f765041@syzkaller.appspotmail.com
Reported-by: syzbot+276ca1c77a19977c0130@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/endpointola.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index e358437..69cebb2 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -118,10 +118,6 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
/* Initialize the bind addr area */
sctp_bind_addr_init(&ep->base.bind_addr, 0);
- /* Remember who we are attached to. */
- ep->base.sk = sk;
- sock_hold(ep->base.sk);
-
/* Create the lists of associations. */
INIT_LIST_HEAD(&ep->asocs);
@@ -154,6 +150,10 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
ep->prsctp_enable = net->sctp.prsctp_enable;
ep->reconf_enable = net->sctp.reconf_enable;
+ /* Remember who we are attached to. */
+ ep->base.sk = sk;
+ sock_hold(ep->base.sk);
+
return ep;
nomem_shkey:
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-24 16:21 UTC (permalink / raw)
To: Johannes Berg, Arnd Bergmann
Cc: abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
Dan Williams, DTML, Eric Caruso, evgreen, Ilias Apalodimas,
Linux ARM, linux-arm-msm, Linux Kernel Mailing List, linux-soc,
Networking, Subash Abhinov Kasiviswanathan, syadagir
In-Reply-To: <b90977f94df020986c6bb490e7fd0262603726b0.camel@sipsolutions.net>
On 6/18/19 2:22 PM, Johannes Berg wrote:
> On Tue, 2019-06-18 at 09:00 -0500, Alex Elder wrote:
. . .
> Anyway, I think for now we could probably live with not having this
> configurable for the IPA driver, and if it *does* need to be
> configurable, it seems like it should be a driver configuration, not a
> channel configuration - so something like a debugfs hook if you really
> just need to play with it for performance testing, or a module
> parameter, or something else?
>
> Or even, in the WWAN framework, a knob that we provide there for the
> WWAN device, rather than for the (newly created) channel.
Agreed. I think a knob is appropriate, it's just a question of how
that control exposed. Same answer to your question below.
-Alex
>> The hardware is capable of aggregating QMAP packets
>> arriving on a connection into a single buffer, so this provides
>> a way of requesting it do that.
>>
>>>> #define RMNET_FLAGS_INGRESS_MAP_COMMANDS (1U << 1)
>>>
>>> Similar here? If you have flow control you probably want to use it?
>>
>> I agree with that, though perhaps there are cases where it
>> is pointless, or can't be supported, so one might want to
>> simply *not* implement/advertise the feature. I don't know.
>
> Sure, but then that's likely something the driver would need to know,
> not necessarily userspace?
>
> johannes
>
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-24 16:21 UTC (permalink / raw)
To: Johannes Berg, Arnd Bergmann
Cc: abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
Dan Williams, DTML, Eric Caruso, evgreen, Ilias Apalodimas,
Linux ARM, linux-arm-msm, Linux Kernel Mailing List, linux-soc,
Networking, Subash Abhinov Kasiviswanathan, syadagir
In-Reply-To: <b23a83c18055470c5308fcd1eed018056371fc1d.camel@sipsolutions.net>
On 6/18/19 2:14 PM, Johannes Berg wrote:
> On Tue, 2019-06-18 at 08:16 -0500, Alex Elder wrote:
>> On 6/17/19 6:28 AM, Johannes Berg wrote:
>>> On Tue, 2019-06-11 at 13:56 +0200, Arnd Bergmann wrote:
>>>> On Tue, Jun 11, 2019 at 10:12 AM Johannes Berg
>>>> <johannes@sipsolutions.net> wrote:
>>>>
>>>>>> As I've made clear before, my work on this has been focused on the IPA transport,
. . .
>> The IPA driver was very large, and in an effort to have an initial driver
>> that was more easily accepted upstream, it was carved down to support
>> a single, very simple use case. It supports only a single channel for
>> carrying network data, and does not expose any of the IPA's other
>> capabilities like filtering and routing (and multiplexing).
>
> Ok. But it *does* use (or even require using) rmnet, so it has multiple
> channels in a sense, no?
Yes. It's a multiplexing protocol, supporting one *or more* channels.
>> Originally the IPA code had an IOCTL interface for adding and removing
>> multiplexed channel IDs, but the simplified use case expected only one
>> channel to be used.
>
> What did those channels do? Create different netdevs? Something else
I don't know. The code I started with only supported the use of
one channel, but assumed the use of rmnet anyway (for aggregation
and checksum offload most likely).
. . .
>> So getting back to your question, the IPA in its current form only
>> has a single "multiplexed" channel carried over the connection
>> between the AP and modem. Previously (and in the future) there
>> was a way to add or remove channels.
>
> What would those channels do?
>
> I've not really been very clear with the differentiation between a
> channel and what's multiplexed inside of the channel.
>
> Using the terminology you defined in your other mail, are you saying
> that IPA (originally) allowed multiple *connections* to the device, or
> is there basically just one connection, with multiple (QMAP-muxed)
> *channels* on top of it?
One connection, with the ability to have multiple multiplexed
channels.
> If the latter, why did IPA need ioctls, rather than rmnet?
The "full" IPA driver supported a lot more than what is being
proposed right now. The strategy for getting support upstream
was to drastically reduce the size of the driver by focusing
on a single use case: providing a netdev data interface served
by the modem.
But even to support that, the IPA driver needed to allow user
space to identify certain resources that need to be used for
implementing that netdev, or configuring whether certain
features (e.g. download checksum) were to be used.
. . .
>> The hardware can aggregate multiple packets received from the
>> modem into a single buffer, which the rmnet driver is then able
>> to deaggregate.
>
> Right, I gathered that much, but I'm not really sure I see why userspace
> would even be allowed to control this? Either the device is doing it or
> not, but the driver is going to have to cope either way?
Maybe because doing aggregation or not is a policy decision?
And/or a tunable parameter? There might be a more appropriate
way to do this.
. . .
>> My goal continues to be getting a baseline IPA driver accepted
>> upstream as soon as possible, so I can then start building on
>> that foundation.
>
> Yeah. My goal is actually the same, but for the Intel driver, but I
> don't have much code yet (it's being cleaned up now) :-)
Well then I guess I'll beat you to it (or I *hope* I do)...
-Alex
>
> johannes
>
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-24 16:21 UTC (permalink / raw)
To: Johannes Berg, Arnd Bergmann, Dan Williams
Cc: Subash Abhinov Kasiviswanathan, abhishek.esse, Ben Chan,
Bjorn Andersson, cpratapa, David Miller, DTML, Eric Caruso,
evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <abdfc6b3a9981bcdef40f85f5442a425ce109010.camel@sipsolutions.net>
On 6/18/19 2:03 PM, Johannes Berg wrote:
> On Tue, 2019-06-18 at 08:45 -0500, Alex Elder wrote:
>
>> If it had a well-defined way of creating new channels to be
>> multiplexed over the connection to the modem, the IPA driver
>> (rather than the rmnet driver) could present network interfaces
>> for each and perform the multiplexing.
>
> Right. That's what I was thinking of.
. . .
>> But I think the IPA driver would register with the WWAN core as
>> a "provider," and then the WWAN core would subsequently request
>> that it instantiate netdevices to represent channels on demand
>> (rather than registering them).
>
> Yeah, I guess you could call it that way.
>
> Really there are two possible ways (and they intersect to some extent).
>
> One is the whole multi-function device, where a single WWAN device is
> composed of channels offered by actually different drivers, e.g. for a
> typical USB device you might have something like cdc_ether and the
> usb_wwan TTY driver. In this way, we need to "compose" the WWAN device
> similarly, e.g. by using the underlying USB device "struct device"
> pointer to tie it together.
I *think* this model makes the most sense. But at this point
it would take very little to convince me otherwise... (And then
I saw Arnd's message advocating the other one, unfortunately...)
> The other is something like IPA or the Intel modem driver, where the
> device is actually a single (e.g. PCIe) device and just has a single
> driver, but that single driver offers different channels.
What I don't like about this is that it's more monolithic. It
seems better to have the low-level IPA or Intel modem driver (or
any other driver that can support communication between the AP
and WWAN device) present communication paths that other function-
specific drivers can attach to and use.
> Now, it's not clear to me where IPA actually falls, because so far we've
> been talking about the IPA driver only as providing *netdevs*, not any
> control channels, so I'm not actually sure where the control channel is.
There is user space code that handles all of this, and as far as I
can tell, parts of it will always remain proprietary.
> For the Intel device, however, the control channel is definitely
> provided by exactly the same driver as the data channels (netdevs).
I do see the need for a control interface. But I suspect it
would *overlap* with what you describe and might need to be more
general and/or extensible. Are there control channels specific to
use for a modem--like a "modem control interface" or something?
Is there something broader, like "this WWAN device supports
functions A, and B with protocols X, Y; please open a connection
to A with protocol X." Do both exist? I'm just trying to contain
whatever a "control channel" really represents, and what it would
be associated with.
> "provider" is a good word, and in fact the Intel driver would also be a
> provider for a GNSS channel (TBD how to represent, a tty?), one or
> multiple debug/tracing channels, data channels (netdevs), AT command
> channels (mbim, ...?) (again tbd how to represent, ttys?), etc.
Yes, this is much clearer to me now.
> What I showed in the header files I posted so far was the provider only
> having "data channel" ops (create/remove a netdev) but for each channel
> type we either want a new method there, or we just change the method to
> be something like
>
> int (*create_channel)(..., enum wwan_chan_type chan_type, ...);
>
> and simply require that the channel is attached to the wwan device with
> the representation-specific call (wwan_attach_netdev, wwan_attach_tty,
> ...).
Or maybe have the WWAN device present interfaces with attributes,
and have drivers that are appropriate for each interface attach
to only the ones they recognize they support.
-Alex
> This is a bit less comfortable because then it's difficult to know what
> was actually created upon the request, so it's probably better to have
> different methods for the different types of representations (like I had
> - add_netdev, add_tty, ...).
>
> Note also that I said "representation-specific", while passing a
> "channel type", so for this we'd actually need a convention on what
> channel type has what kind of representation, which again gets awkward.
> Better to make it explicit.
>
> (And even then, we might be able to let userspace have some control,
> e.g. the driver might be able to create a debug channel as both a TTY or
> something else)
>
> johannes
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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 NNTP newsgroup(s).