Recent changes to kernel_connect() and kernel_bind() ensure that callers are insulated from changes to the address parameter made by BPF SOCK_ADDR hooks. This patch wraps direct calls to ops->connect() and ops->bind() with kernel_connect() and kernel_bind() to ensure that SMB mounts do not see their mount address overwritten in such cases.
Link: https://lore.kernel.org/netdev/9944248dba1bce861375fcce9de663934d933ba9.came... Cc: stable@vger.kernel.org # 6.x.y Signed-off-by: Jordan Rife jrife@google.com --- fs/smb/client/connect.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 3902e90dca6b0..ce11165446cfb 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -2895,9 +2895,9 @@ bind_socket(struct TCP_Server_Info *server) if (server->srcaddr.ss_family != AF_UNSPEC) { /* Bind to the specified local IP address */ struct socket *socket = server->ssocket; - rc = socket->ops->bind(socket, - (struct sockaddr *) &server->srcaddr, - sizeof(server->srcaddr)); + rc = kernel_bind(socket, + (struct sockaddr *) &server->srcaddr, + sizeof(server->srcaddr)); if (rc < 0) { struct sockaddr_in *saddr4; struct sockaddr_in6 *saddr6; @@ -3046,8 +3046,8 @@ generic_ip_connect(struct TCP_Server_Info *server) socket->sk->sk_sndbuf, socket->sk->sk_rcvbuf, socket->sk->sk_rcvtimeo);
- rc = socket->ops->connect(socket, saddr, slen, - server->noblockcnt ? O_NONBLOCK : 0); + rc = kernel_connect(socket, saddr, slen, + server->noblockcnt ? O_NONBLOCK : 0); /* * When mounting SMB root file systems, we do not want to block in * connect. Otherwise bail out and then let cifs_reconnect() perform
Jordan Rife jrife@google.com writes:
Recent changes to kernel_connect() and kernel_bind() ensure that callers are insulated from changes to the address parameter made by BPF SOCK_ADDR hooks. This patch wraps direct calls to ops->connect() and ops->bind() with kernel_connect() and kernel_bind() to ensure that SMB mounts do not see their mount address overwritten in such cases.
Link: https://lore.kernel.org/netdev/9944248dba1bce861375fcce9de663934d933ba9.came... Cc: stable@vger.kernel.org # 6.x.y Signed-off-by: Jordan Rife jrife@google.com
fs/smb/client/connect.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Paulo Alcantara (SUSE) pc@manguebit.com
tentatively merged into cifs-2.6.git for-next pending testing and additional review
On Wed, Oct 4, 2023 at 10:44 AM Paulo Alcantara pc@manguebit.com wrote:
Jordan Rife jrife@google.com writes:
Recent changes to kernel_connect() and kernel_bind() ensure that callers are insulated from changes to the address parameter made by BPF SOCK_ADDR hooks. This patch wraps direct calls to ops->connect() and ops->bind() with kernel_connect() and kernel_bind() to ensure that SMB mounts do not see their mount address overwritten in such cases.
Link: https://lore.kernel.org/netdev/9944248dba1bce861375fcce9de663934d933ba9.came... Cc: stable@vger.kernel.org # 6.x.y Signed-off-by: Jordan Rife jrife@google.com
fs/smb/client/connect.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Paulo Alcantara (SUSE) pc@manguebit.com
On Wed, Oct 4, 2023 at 10:15 AM Steve French smfrench@gmail.com wrote:
tentatively merged into cifs-2.6.git for-next pending testing and additional review
On Wed, Oct 4, 2023 at 10:44 AM Paulo Alcantara pc@manguebit.com wrote:
Jordan Rife jrife@google.com writes:
Recent changes to kernel_connect() and kernel_bind() ensure that callers are insulated from changes to the address parameter made by BPF SOCK_ADDR hooks. This patch wraps direct calls to ops->connect() and ops->bind() with kernel_connect() and kernel_bind() to ensure that SMB mounts do not see their mount address overwritten in such cases.
Link: https://lore.kernel.org/netdev/9944248dba1bce861375fcce9de663934d933ba9.came... Cc: stable@vger.kernel.org # 6.x.y Signed-off-by: Jordan Rife jrife@google.com
fs/smb/client/connect.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Paulo Alcantara (SUSE) pc@manguebit.com
-- Thanks,
Steve
Do you want this to go through the cifs tree?
Yes. This was originally a part of a larger patch set destined for the net tree (https://lore.kernel.org/netdev/20230919175159.144073-1-jrife@google.com/T/#u). It was ultimately decided (https://lore.kernel.org/netdev/20230919175323.144902-1-jrife@google.com/T/#m...) over there to try sending patches to the appropriate trees to avoid merge conflicts.
How urgent do you think it is (or should it wait for 6.7)?
Not super urgent, but ultimately it should be backported to stable kernels 4.19+ (all versions where it's possible to overwrite the address parameter with BPF hooks). The risk here is in environments where BPF hooks are used to rewrite the connect/bind addresses (common in systems like Kubernetes w/ Cilium). Doing so can break your mounts, since the original mount address is "forgotten" when a call to ops->connect() overwrites it (have confirmed this scenario myself). IME, this scenario is more common to see with NFS mounts, but still possible with SMB.
- Jordan
linux-stable-mirror@lists.linaro.org
-
Jordan Rife -
Paulo Alcantara -
Steve French