net/atm/common.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
dev->ci_range.vpi_bits and vci_bits can legitimately be ATM_CI_MAX (-1),
a sentinel meaning "no range configured, use maximum" (see
include/uapi/linux/atmdev.h). Some drivers, such as usbatm_atm_init()
in drivers/usb/atm/usbatm.c, set this sentinel and never resolve it to
an actual bit width.
__vcc_connect() uses these fields directly as a shift amount without
checking for the sentinel, so binding a PVC socket on such a device
triggers a negative shift:
UBSAN: shift-out-of-bounds in net/atm/common.c:381:10
shift exponent -1 is negative
Skip the range check when ci_range.vpi_bits/vci_bits is still
ATM_CI_MAX, since that value means "unrestricted".
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+6665d3db5fef15914802@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6665d3db5fef15914802
Tested-by: syzbot+6665d3db5fef15914802@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
net/atm/common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/atm/common.c b/net/atm/common.c
index 81195727fa18..73057bb016f7 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -378,8 +378,11 @@ static int __vcc_connect(struct atm_vcc *vcc, struct atm_dev *dev, short vpi,
int error;
if ((vpi != ATM_VPI_UNSPEC && vpi != ATM_VPI_ANY &&
+ dev->ci_range.vpi_bits != ATM_CI_MAX &&
vpi >> dev->ci_range.vpi_bits) || (vci != ATM_VCI_UNSPEC &&
- vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
+ vci != ATM_VCI_ANY &&
+ dev->ci_range.vci_bits != ATM_CI_MAX &&
+ vci >> dev->ci_range.vci_bits))
return -EINVAL;
if (vci > 0 && vci < ATM_NOT_RSV_VCI && !capable(CAP_NET_BIND_SERVICE))
return -EPERM;
--
2.34.1This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824024620.23485-1-kartikey406@gmail.com?part=1
---
net: atm: fix shift-out-of-bounds in __vcc_connect()
__vcc_connect() used dev->ci_range.vpi_bits and vci_bits directly as shift
amounts, which is a negative shift when those fields still hold the
ATM_CI_MAX (-1) sentinel. The patch skips the VPI/VCI range check
entirely when either field equals ATM_CI_MAX.
Closes: https://syzkaller.appspot.com/bug?extid=6665d3db5fef15914802
> diff --git a/net/atm/common.c b/net/atm/common.c
> index 81195727fa18b..b38359453555b 100644
> --- a/net/atm/common.c
> +++ b/net/atm/common.c
> @@ -378,8 +378,11 @@ static int __vcc_connect(struct atm_vcc *vcc, struct atm_dev *dev, short vpi,
> int error;
>
> if ((vpi != ATM_VPI_UNSPEC && vpi != ATM_VPI_ANY &&
> + dev->ci_range.vpi_bits != ATM_CI_MAX &&
> vpi >> dev->ci_range.vpi_bits) || (vci != ATM_VCI_UNSPEC &&
> - vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
> + vci != ATM_VCI_ANY &&
> + dev->ci_range.vci_bits != ATM_CI_MAX &&
> + vci >> dev->ci_range.vci_bits))
> return -EINVAL;
[Severity: High]
This isn't a bug introduced by this patch, but does the same negative shift
remain reachable from the function being modified here? find_ci() is
called by __vcc_connect() a few lines below:
net/atm/common.c:__vcc_connect() {
...
if (test_bit(ATM_DF_REMOVED, &dev->flags) ||
(error = find_ci(vcc, &vpi, &vci))) {
...
}
and find_ci() shifts by the same fields with no sentinel check:
net/atm/common.c:find_ci() {
...
else if (p >= 1 << vcc->dev->ci_range.vpi_bits)
p = 0;
if (*vci != ATM_VCI_ANY)
c = *vci;
else if (c < ATM_NOT_RSV_VCI || c >= 1 << vcc->dev->ci_range.vci_bits)
c = ATM_NOT_RSV_VCI;
...
if (*vci == ATM_VCI_ANY) {
c++;
if (c >= 1 << vcc->dev->ci_range.vci_bits)
c = ATM_NOT_RSV_VCI;
}
...
p++;
if (p >= 1 << vcc->dev->ci_range.vpi_bits)
p = 0;
...
}
Note the new guards in __vcc_connect() are skipped when vpi is ATM_VPI_ANY
or vci is ATM_VCI_ANY (both -1 in include/uapi/linux/atm.h), which is
exactly the case that then reaches find_ci(). pvc_bind() passes the
userspace sockaddr_atmpvc values through vcc_connect() unchanged, and
vcc_connect() only rejects !(vpi || vci):
net/atm/pvc.c:pvc_bind() {
...
error = vcc_connect(sock, addr->sap_addr.itf, addr->sap_addr.vpi,
addr->sap_addr.vci);
...
}
On a device that leaves the sentinel in place, e.g. usbatm_atm_init():
drivers/usb/atm/usbatm.c:usbatm_atm_init() {
...
atm_dev->ci_range.vpi_bits = ATM_CI_MAX;
atm_dev->ci_range.vci_bits = ATM_CI_MAX;
...
}
wouldn't a bind() with sap_addr.vpi or sap_addr.vci set to -1 still produce
the same "shift exponent -1 is negative" report, only from find_ci()
instead of __vcc_connect()? With panic_on_warn set, ubsan_epilogue()
calls check_panic_on_warn("UBSAN"), so this would still be a panic.
There also looks to be a secondary effect: since 1 << -1 folds to INT_MIN
on architectures that mask the shift count, both p >= 1 << vpi_bits and
c >= 1 << vci_bits are always true, so after the first wildcard VCC is
allocated the do/while scan in find_ci() hits old_p == p && old_c == c on
the first iteration and returns -EADDRINUSE. Does that make a second
automatically-allocated VC impossible on such a device?
Would it be preferable to resolve the sentinel to a real bit width once,
so both __vcc_connect() and find_ci() see a sane value?
[Severity: Medium]
Separately, is skipping the check the intended reading of the sentinel?
The uAPI describes it as the maximum range rather than as unrestricted:
include/uapi/linux/atmdev.h:
#define ATM_CI_MAX -1 /* use maximum range of VPI/VCI */
struct atm_cirange {
signed char vpi_bits; /* 1..8, ATM_CI_MAX (-1) for maximum */
signed char vci_bits; /* 1..16, ATM_CI_MAX (-1) for maximum */
};
with the maxima defined in include/uapi/linux/atm.h as ATM_MAX_VPI 255 and
ATM_MAX_VCI 65535. The commit message says:
"Skip the range check when ci_range.vpi_bits/vci_bits is still
ATM_CI_MAX, since that value means "unrestricted"."
Should this instead clamp to 8/16 bits (or compare against ATM_MAX_VPI /
ATM_MAX_VCI) and keep the check?
This also appears to change behaviour for negative identifiers other than
the ANY/UNSPEC magic values. Before the patch, vpi >> -1 folded to
vpi >> 31 on architectures that mask the shift count, which is -1 for a
negative vpi and therefore returned -EINVAL. After the patch a bind() with
sap_addr.vpi = -5 or sap_addr.vci = -65531 is accepted, since vcc_connect()
only rejects !(vpi || vci) and check_ci() masks vci purely for hashing:
net/atm/common.c:check_ci() {
struct hlist_head *head = &vcc_hash[vci & (VCC_HTABLE_SIZE - 1)];
...
}
The raw value then reaches vcc->vpi/vcc->vci and dev->ops->open().
usbatm_atm_open() does not bound them, and usbatm_write_cells() packs them
into the fixed-width cell header without masking:
drivers/usb/atm/usbatm.c:usbatm_write_cells() {
...
ptr[0] = vcc->vpi >> 4;
ptr[1] = (vcc->vpi << 4) | (vcc->vci >> 12);
ptr[2] = vcc->vci >> 4;
ptr[3] = vcc->vci << 4;
...
}
Can this alias distinct VCCs onto the same on-the-wire VPI/VCI, for example
vci = -65531 and vci = 65541 both transmitting on reserved VCI 5, while
the check
if (vci > 0 && vci < ATM_NOT_RSV_VCI && !capable(CAP_NET_BIND_SERVICE))
return -EPERM;
is evaluated on the untruncated value and so does not fire? The duplicate
detection in check_ci() compares the untruncated values too, so it would
not catch the collision either.
On Wed, Aug 26, 2026 at 1:59 PM Simon Horman <horms@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824024620.23485-1-kartikey406@gmail.com?part=1
> ---
> net: atm: fix shift-out-of-bounds in __vcc_connect()
>
> __vcc_connect() used dev->ci_range.vpi_bits and vci_bits directly as shift
> amounts, which is a negative shift when those fields still hold the
> ATM_CI_MAX (-1) sentinel. The patch skips the VPI/VCI range check
> entirely when either field equals ATM_CI_MAX.
>
> Closes: https://syzkaller.appspot.com/bug?extid=6665d3db5fef15914802
A simpler fix would be something like
usb: atm: usbatm: fix invalid ci_range initialization
syzbot reported a shift-out-of-bounds in __vcc_connect():
UBSAN: shift-out-of-bounds in net/atm/common.c:382:32
shift exponent -1 is negative
CPU: 0 UID: 0 PID: 5987 Comm: syz.0.18 Not tainted syzkaller #0
PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute
Engine, BIOS Google 08/05/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
ubsan_epilogue+0xa/0x30 lib/ubsan.c:233
__ubsan_handle_shift_out_of_bounds+0x36d/0x400 lib/ubsan.c:494
__vcc_connect+0x14b4/0x19c0 net/atm/common.c:382
vcc_connect+0x328/0x8f0 net/atm/common.c:498
pvc_bind+0x272/0x380 net/atm/pvc.c:52
__sys_bind+0x2e3/0x410 net/socket.c:1976
__x64_sys_bind+0x7a/0x90 net/socket.c:1979
...
ATM device ci_range fields (vpi_bits and vci_bits) represent the number
of bits supported for VPI and VCI addressing on the device.
net/atm/common.c directly uses these fields as bit shift counts:
vpi >> dev->ci_range.vpi_bits
vci >> dev->ci_range.vci_bits
1 << vcc->dev->ci_range.vpi_bits
1 << vcc->dev->ci_range.vci_bits
usbatm_atm_init() sets ci_range.vpi_bits and ci_range.vci_bits to
ATM_CI_MAX (-1), which was defined in <uapi/linux/atmdev.h> as a sentinel
value for user-space ATM_SETCIRANGE requests, not as a valid bit count.
Shifting by -1 is undefined behavior and triggers UBSAN warnings.
ATM UNI cell headers allow up to 8 bits for VPI (0..255) and 16 bits
for VCI (0..65535).
Initialize vpi_bits to 8 and vci_bits to 16, as done by solos-pci.
...
diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index 9600e1ec099304e465dddb00c36817339f538b3b..7b0c791399eaac9ebe00541a2c41d715f465a24f
100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -917,8 +917,8 @@ static int usbatm_atm_init(struct usbatm_data *instance)
instance->atm_dev = atm_dev;
- atm_dev->ci_range.vpi_bits = ATM_CI_MAX;
- atm_dev->ci_range.vci_bits = ATM_CI_MAX;
+ atm_dev->ci_range.vpi_bits = 8;
+ atm_dev->ci_range.vci_bits = 16;
atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
/* temp init ATM device, set to 128kbit */
On Wed, Aug 26, 2026 at 5:34 PM Eric Dumazet <edumazet@google.com> wrote: > > On Wed, Aug 26, 2026 at 1:59 PM Simon Horman <horms@kernel.org> wrote: > > > > This is an AI-generated review of your patch. The human sending this > > email has considered the AI review valid, or at least plausible. > > Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824024620.23485-1-kartikey406@gmail.com?part=1 > > --- > > net: atm: fix shift-out-of-bounds in __vcc_connect() > > > > __vcc_connect() used dev->ci_range.vpi_bits and vci_bits directly as shift > > amounts, which is a negative shift when those fields still hold the > > ATM_CI_MAX (-1) sentinel. The patch skips the VPI/VCI range check > > entirely when either field equals ATM_CI_MAX. > > > > Closes: https://syzkaller.appspot.com/bug?extid=6665d3db5fef15914802 > > A simpler fix would be something like > > usb: atm: usbatm: fix invalid ci_range initialization > > syzbot reported a shift-out-of-bounds in __vcc_connect(): > > UBSAN: shift-out-of-bounds in net/atm/common.c:382:32 > shift exponent -1 is negative > CPU: 0 UID: 0 PID: 5987 Comm: syz.0.18 Not tainted syzkaller #0 > PREEMPT(full) > Hardware name: Google Google Compute Engine/Google Compute > Engine, BIOS Google 08/05/2026 > Call Trace: > <TASK> > dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120 > ubsan_epilogue+0xa/0x30 lib/ubsan.c:233 > __ubsan_handle_shift_out_of_bounds+0x36d/0x400 lib/ubsan.c:494 > __vcc_connect+0x14b4/0x19c0 net/atm/common.c:382 > vcc_connect+0x328/0x8f0 net/atm/common.c:498 > pvc_bind+0x272/0x380 net/atm/pvc.c:52 > __sys_bind+0x2e3/0x410 net/socket.c:1976 > __x64_sys_bind+0x7a/0x90 net/socket.c:1979 > ... > > ATM device ci_range fields (vpi_bits and vci_bits) represent the number > of bits supported for VPI and VCI addressing on the device. > net/atm/common.c directly uses these fields as bit shift counts: > vpi >> dev->ci_range.vpi_bits > vci >> dev->ci_range.vci_bits > 1 << vcc->dev->ci_range.vpi_bits > 1 << vcc->dev->ci_range.vci_bits > > usbatm_atm_init() sets ci_range.vpi_bits and ci_range.vci_bits to > ATM_CI_MAX (-1), which was defined in <uapi/linux/atmdev.h> as a sentinel > value for user-space ATM_SETCIRANGE requests, not as a valid bit count. > Shifting by -1 is undefined behavior and triggers UBSAN warnings. > > ATM UNI cell headers allow up to 8 bits for VPI (0..255) and 16 bits > for VCI (0..65535). > Initialize vpi_bits to 8 and vci_bits to 16, as done by solos-pci. > > ... > > > diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c > index 9600e1ec099304e465dddb00c36817339f538b3b..7b0c791399eaac9ebe00541a2c41d715f465a24f > 100644 > --- a/drivers/usb/atm/usbatm.c > +++ b/drivers/usb/atm/usbatm.c > @@ -917,8 +917,8 @@ static int usbatm_atm_init(struct usbatm_data *instance) > > instance->atm_dev = atm_dev; > > - atm_dev->ci_range.vpi_bits = ATM_CI_MAX; > - atm_dev->ci_range.vci_bits = ATM_CI_MAX; > + atm_dev->ci_range.vpi_bits = 8; > + atm_dev->ci_range.vci_bits = 16; > atm_dev->signal = ATM_PHY_SIG_UNKNOWN; > > /* temp init ATM device, set to 128kbit */ Initially, I was thinking the same way. Thanks for reviewing it. I will send patch v2 shortly Thanks Deepanshu
© 2016 - 2026 Red Hat, Inc.