Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ax25: Remove broken autobind

Binding AX25 socket by using the autobind feature leads to memory leaks
in ax25_connect() and also refcount leaks in ax25_release(). Memory
leak was detected with kmemleak:

================================================================
unreferenced object 0xffff8880253cd680 (size 96):
backtrace:
__kmalloc_node_track_caller_noprof (./include/linux/kmemleak.h:43)
kmemdup_noprof (mm/util.c:136)
ax25_rt_autobind (net/ax25/ax25_route.c:428)
ax25_connect (net/ax25/af_ax25.c:1282)
__sys_connect_file (net/socket.c:2045)
__sys_connect (net/socket.c:2064)
__x64_sys_connect (net/socket.c:2067)
do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
================================================================

When socket is bound, refcounts must be incremented the way it is done
in ax25_bind() and ax25_setsockopt() (SO_BINDTODEVICE). In case of
autobind, the refcounts are not incremented.

This bug leads to the following issue reported by Syzkaller:

================================================================
ax25_connect(): syz-executor318 uses autobind, please contact jreuter@yaina.de
------------[ cut here ]------------
refcount_t: decrement hit 0; leaking memory.
WARNING: CPU: 0 PID: 5317 at lib/refcount.c:31 refcount_warn_saturate+0xfa/0x1d0 lib/refcount.c:31
Modules linked in:
CPU: 0 UID: 0 PID: 5317 Comm: syz-executor318 Not tainted 6.14.0-rc4-syzkaller-00278-gece144f151ac #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:refcount_warn_saturate+0xfa/0x1d0 lib/refcount.c:31
...
Call Trace:
<TASK>
__refcount_dec include/linux/refcount.h:336 [inline]
refcount_dec include/linux/refcount.h:351 [inline]
ref_tracker_free+0x6af/0x7e0 lib/ref_tracker.c:236
netdev_tracker_free include/linux/netdevice.h:4302 [inline]
netdev_put include/linux/netdevice.h:4319 [inline]
ax25_release+0x368/0x960 net/ax25/af_ax25.c:1080
__sock_release net/socket.c:647 [inline]
sock_close+0xbc/0x240 net/socket.c:1398
__fput+0x3e9/0x9f0 fs/file_table.c:464
__do_sys_close fs/open.c:1580 [inline]
__se_sys_close fs/open.c:1565 [inline]
__x64_sys_close+0x7f/0x110 fs/open.c:1565
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
...
</TASK>
================================================================

Considering the issues above and the comments left in the code that say:
"check if we can remove this feature. It is broken."; "autobinding in this
may or may not work"; - it is better to completely remove this feature than
to fix it because it is broken and leads to various kinds of memory bugs.

Now calling connect() without first binding socket will result in an
error (-EINVAL). Userspace software that relies on the autobind feature
might get broken. However, this feature does not seem widely used with
this specific driver as it was not reliable at any point of time, and it
is already broken anyway. E.g. ax25-tools and ax25-apps packages for
popular distributions do not use the autobind feature for AF_AX25.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+33841dc6aa3e1d86b78a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=33841dc6aa3e1d86b78a
Signed-off-by: Murad Masimov <m.masimov@mt-integration.ru>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Murad Masimov and committed by
David S. Miller
2f6efbab ed3ba9b6

+10 -95
-1
include/net/ax25.h
··· 418 418 int ax25_rt_ioctl(unsigned int, void __user *); 419 419 extern const struct seq_operations ax25_rt_seqops; 420 420 ax25_route *ax25_get_route(ax25_address *addr, struct net_device *dev); 421 - int ax25_rt_autobind(ax25_cb *, ax25_address *); 422 421 struct sk_buff *ax25_rt_build_path(struct sk_buff *, ax25_address *, 423 422 ax25_address *, ax25_digi *); 424 423 void ax25_rt_free(void);
+10 -20
net/ax25/af_ax25.c
··· 1270 1270 } 1271 1271 } 1272 1272 1273 - /* 1274 - * Must bind first - autobinding in this may or may not work. If 1275 - * the socket is already bound, check to see if the device has 1276 - * been filled in, error if it hasn't. 1277 - */ 1273 + /* Must bind first - autobinding does not work. */ 1278 1274 if (sock_flag(sk, SOCK_ZAPPED)) { 1279 - /* check if we can remove this feature. It is broken. */ 1280 - printk(KERN_WARNING "ax25_connect(): %s uses autobind, please contact jreuter@yaina.de\n", 1281 - current->comm); 1282 - if ((err = ax25_rt_autobind(ax25, &fsa->fsa_ax25.sax25_call)) < 0) { 1283 - kfree(digi); 1284 - goto out_release; 1285 - } 1275 + kfree(digi); 1276 + err = -EINVAL; 1277 + goto out_release; 1278 + } 1286 1279 1287 - ax25_fillin_cb(ax25, ax25->ax25_dev); 1288 - ax25_cb_add(ax25); 1289 - } else { 1290 - if (ax25->ax25_dev == NULL) { 1291 - kfree(digi); 1292 - err = -EHOSTUNREACH; 1293 - goto out_release; 1294 - } 1280 + /* Check to see if the device has been filled in, error if it hasn't. */ 1281 + if (ax25->ax25_dev == NULL) { 1282 + kfree(digi); 1283 + err = -EHOSTUNREACH; 1284 + goto out_release; 1295 1285 } 1296 1286 1297 1287 if (sk->sk_type == SOCK_SEQPACKET &&
-74
net/ax25/ax25_route.c
··· 373 373 return ax25_rt; 374 374 } 375 375 376 - /* 377 - * Adjust path: If you specify a default route and want to connect 378 - * a target on the digipeater path but w/o having a special route 379 - * set before, the path has to be truncated from your target on. 380 - */ 381 - static inline void ax25_adjust_path(ax25_address *addr, ax25_digi *digipeat) 382 - { 383 - int k; 384 - 385 - for (k = 0; k < digipeat->ndigi; k++) { 386 - if (ax25cmp(addr, &digipeat->calls[k]) == 0) 387 - break; 388 - } 389 - 390 - digipeat->ndigi = k; 391 - } 392 - 393 - 394 - /* 395 - * Find which interface to use. 396 - */ 397 - int ax25_rt_autobind(ax25_cb *ax25, ax25_address *addr) 398 - { 399 - ax25_uid_assoc *user; 400 - ax25_route *ax25_rt; 401 - int err = 0; 402 - 403 - ax25_route_lock_use(); 404 - ax25_rt = ax25_get_route(addr, NULL); 405 - if (!ax25_rt) { 406 - ax25_route_lock_unuse(); 407 - return -EHOSTUNREACH; 408 - } 409 - rcu_read_lock(); 410 - if ((ax25->ax25_dev = ax25_dev_ax25dev(ax25_rt->dev)) == NULL) { 411 - err = -EHOSTUNREACH; 412 - goto put; 413 - } 414 - 415 - user = ax25_findbyuid(current_euid()); 416 - if (user) { 417 - ax25->source_addr = user->call; 418 - ax25_uid_put(user); 419 - } else { 420 - if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) { 421 - err = -EPERM; 422 - goto put; 423 - } 424 - ax25->source_addr = *(ax25_address *)ax25->ax25_dev->dev->dev_addr; 425 - } 426 - 427 - if (ax25_rt->digipeat != NULL) { 428 - ax25->digipeat = kmemdup(ax25_rt->digipeat, sizeof(ax25_digi), 429 - GFP_ATOMIC); 430 - if (ax25->digipeat == NULL) { 431 - err = -ENOMEM; 432 - goto put; 433 - } 434 - ax25_adjust_path(addr, ax25->digipeat); 435 - } 436 - 437 - if (ax25->sk != NULL) { 438 - local_bh_disable(); 439 - bh_lock_sock(ax25->sk); 440 - sock_reset_flag(ax25->sk, SOCK_ZAPPED); 441 - bh_unlock_sock(ax25->sk); 442 - local_bh_enable(); 443 - } 444 - 445 - put: 446 - rcu_read_unlock(); 447 - ax25_route_lock_unuse(); 448 - return err; 449 - } 450 376 451 377 struct sk_buff *ax25_rt_build_path(struct sk_buff *skb, ax25_address *src, 452 378 ax25_address *dest, ax25_digi *digi)