Re: [PATCH bpf 1/5] tcp, ulp: add alias for all ulp modules

Song Liu
2018-08-17T00:25:57Z

On Thu, Aug 16, 2018 at 12:49 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Lets not turn the TCP ULP lookup into an arbitrary module loader as
> we only intend to load ULP modules through this mechanism, not other
> unrelated kernel modules:
>
> [root@bar]# cat foo.c
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <linux/tcp.h>
> #include <linux/in.h>
>
> int main(void)
> {
> int sock = socket(PF_INET, SOCK_STREAM, 0);
> setsockopt(sock, IPPROTO_TCP, TCP_ULP, "sctp", sizeof("sctp"));
> return 0;
> }
>
> [root@bar]# gcc foo.c -O2 -Wall
> [root@bar]# lsmod | grep sctp
> [root@bar]# ./a.out
> [root@bar]# lsmod | grep sctp
> sctp 1077248 4
> libcrc32c 16384 3 nf_conntrack,nf_nat,sctp
> [root@bar]#
>
> Fix it by adding module alias to TCP ULP modules, so probing module
> via request_module() will be limited to tcp-ulp-[name]. The existing
> modules like kTLS will load fine given tcp-ulp-tls alias, but others
> will fail to load:
>
> [root@bar]# lsmod | grep sctp
> [root@bar]# ./a.out
> [root@bar]# lsmod | grep sctp
> [root@bar]#
>
> Sockmap is not affected from this since it's either built-in or not.
>
> Fixes: 734942cc4ea6 ("tcp: ULP infrastructure")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Acked-by: Song Liu <songliubraving@fb.com>
> ---
> include/net/tcp.h | 4 ++++
> net/ipv4/tcp_ulp.c | 2 +-
> net/tls/tls_main.c | 1 +
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d196901..770917d 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2065,6 +2065,10 @@ int tcp_set_ulp_id(struct sock *sk, const int ulp);
> void tcp_get_available_ulp(char *buf, size_t len);
> void tcp_cleanup_ulp(struct sock *sk);
>
> +#define MODULE_ALIAS_TCP_ULP(name) \
> + __MODULE_INFO(alias, alias_userspace, name); \
> + __MODULE_INFO(alias, alias_tcp_ulp, "tcp-ulp-" name)
> +
> /* Call BPF_SOCK_OPS program that returns an int. If the return value
> * is < 0, then the BPF op failed (for example if the loaded BPF
> * program does not support the chosen operation or there is no BPF
> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
> index 622caa4..7dd44b6 100644
> --- a/net/ipv4/tcp_ulp.c
> +++ b/net/ipv4/tcp_ulp.c
> @@ -51,7 +51,7 @@ static const struct tcp_ulp_ops *__tcp_ulp_find_autoload(const char *name)
> #ifdef CONFIG_MODULES
> if (!ulp && capable(CAP_NET_ADMIN)) {
> rcu_read_unlock();
> - request_module("%s", name);
> + request_module("tcp-ulp-%s", name);
> rcu_read_lock();
> ulp = tcp_ulp_find(name);
> }
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index b09867c..93c0c22 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -45,6 +45,7 @@
> MODULE_AUTHOR("Mellanox Technologies");
> MODULE_DESCRIPTION("Transport Layer Security Support");
> MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS_TCP_ULP("tls");
>
> enum {
> TLSV4,
> --
> 2.9.5
>