Skip to content

media: uvcvideo: Skip parsing frames of type UVC_VS_UNDEFINED in uvc_parse_format #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
bmastbergen merged 2 commits into from
Feb 7, 2025
Merged

media: uvcvideo: Skip parsing frames of type UVC_VS_UNDEFINED in uvc_parse_format #107

bmastbergen merged 2 commits into from
Feb 7, 2025

Conversation

@bmastbergen
Copy link
Collaborator

@bmastbergen bmastbergen commented Feb 7, 2025

jira VULN-9667
cve CVE-2024-53104

commit-author Benoit Sevens <bsevens@google.com>
commit ecf2b43018da9579842c774b7f35dbe11b5c38dd

This can lead to out of bounds writes since frames of this type were not taken into account when calculating the size of the frames buffer in uvc_parse_streaming.

Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
	Signed-off-by: Benoit Sevens <bsevens@google.com>
	Cc: stable@vger.kernel.org
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
	Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
(cherry picked from commit ecf2b43018da9579842c774b7f35dbe11b5c38dd)
	Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>

build.log

Kselftest runs before and after:
selftests-before.log
selftests-after.log

brett@lycia ~/ciq/vuln-9667 % grep ^ok selftests-before.log | wc -l
309
brett@lycia ~/ciq/vuln-9667 % grep ^ok selftests-after.log | wc -l
310
brett@lycia ~/ciq/vuln-9667 %

@bmastbergen
media: uvcvideo: Skip parsing frames of type UVC_VS_UNDEFINED in uvc_…
c107f07 
…parse_format

jira VULN-9667
cve CVE-2024-53104
commit-author Benoit Sevens <bsevens@google.com>
commit ecf2b43

This can lead to out of bounds writes since frames of this type were not
taken into account when calculating the size of the frames buffer in
uvc_parse_streaming.

Fixes: c0efd23 ("V4L/DVB (8145a): USB Video Class driver")
	Signed-off-by: Benoit Sevens <bsevens@google.com>
	Cc: stable@vger.kernel.org
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
	Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
(cherry picked from commit ecf2b43)
	Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jason-rodri
jason-rodri previously approved these changes Feb 7, 2025
View reviewed changes
gvrose8192
gvrose8192 previously approved these changes Feb 7, 2025
View reviewed changes
Copy link

@gvrose8192 gvrose8192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending completion of the checks. Otherwise, LGTM - Thanks!

@PlaidCat
Copy link
Collaborator

PlaidCat commented Feb 7, 2025 •
edited
Loading

@bmastbergen could you make an additional commit with this PR
can you change both of these from pull_request_target -> pull_request
https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_aarch64.yml
https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_x86_64.yml

This is an artifact from a misunderstanding of FORKs and github actions.
To commit you need to do git add -f .github/workflows/build-check_aarch64.yml first

@PlaidCat @bmastbergen
github actions: Reduce Pull Request openness
2b64cea 
Since the kernel builds are very expensive we only want to run the
workflows associated with them is by approval of staff / maintainers of
the kernel.  There was a miss understanding initially that
pull_request_target was required to get access to the code.
@bmastbergen bmastbergen dismissed stale reviews from gvrose8192 and jason-rodri via 2b64cea February 7, 2025 17:30
@bmastbergen
Copy link
Collaborator Author

bmastbergen commented Feb 7, 2025

@bmastbergen could you make an additional commit with this PR can you change both of these from pull_request_target -> pull_request https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_aarch64.yml https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_x86_64.yml

This is an artifact from a misunderstanding of FORKs and github actions. To commit you need to do git add -f .github/workflows/build-check_aarch64.yml first

I cherry-picked the change from ciqlts8_8. Hope that is ok.

@PlaidCat
Copy link
Collaborator

PlaidCat commented Feb 7, 2025

@bmastbergen could you make an additional commit with this PR can you change both of these from pull_request_target -> pull_request https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_aarch64.yml https://github.com/ctrliq/kernel-src-tree/blob/ciqlts9_2/.github/workflows/build-check_x86_64.yml
This is an artifact from a misunderstanding of FORKs and github actions. To commit you need to do git add -f .github/workflows/build-check_aarch64.yml first

I cherry-picked the change from ciqlts8_8. Hope that is ok.

Works for me, also we have doubled up builds here because this PR introduces pull_request but pull_request_target is in the target branch no much to do here.

PlaidCat
PlaidCat approved these changes Feb 7, 2025
View reviewed changes
Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@bmastbergen bmastbergen merged commit 26e535e into ciqlts9_2 Feb 7, 2025
5 checks passed
@bmastbergen bmastbergen deleted the bmastbergen_ciqlts9_2/VULN-9667 branch February 7, 2025 19:04
@bmastbergen bmastbergen mentioned this pull request Feb 7, 2025
Merged
github-actions bot pushed a commit that referenced this pull request Jul 16, 2025
bonding: check xdp prog when set bond mode
07f780f 
JIRA: https://issues.redhat.com/browse/RHEL-96600
Upstream Status: net.git commit 094ee60

commit 094ee60
Author: Wang Liang <wangliang74@huawei.com>
Date:   Fri Mar 21 12:48:52 2025 +0800

    bonding: check xdp prog when set bond mode

    Following operations can trigger a warning[1]:

        ip netns add ns1
        ip netns exec ns1 ip link add bond0 type bond mode balance-rr
        ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
        ip netns exec ns1 ip link set bond0 type bond mode broadcast
        ip netns del ns1

    When delete the namespace, dev_xdp_uninstall() is called to remove xdp
    program on bond dev, and bond_xdp_set() will check the bond mode. If bond
    mode is changed after attaching xdp program, the warning may occur.

    Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
    with xdp program attached is not good. Add check for xdp program when set
    bond mode.

        [1]
        ------------[ cut here ]------------
        WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
        Modules linked in:
        CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
        Workqueue: netns cleanup_net
        RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
        Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
        RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
        RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
        RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
        RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
        R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
        R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
        FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
        CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
        CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
        Call Trace:
         <TASK>
         ? __warn+0x83/0x130
         ? unregister_netdevice_many_notify+0x8d9/0x930
         ? report_bug+0x18e/0x1a0
         ? handle_bug+0x54/0x90
         ? exc_invalid_op+0x18/0x70
         ? asm_exc_invalid_op+0x1a/0x20
         ? unregister_netdevice_many_notify+0x8d9/0x930
         ? bond_net_exit_batch_rtnl+0x5c/0x90
         cleanup_net+0x237/0x3d0
         process_one_work+0x163/0x390
         worker_thread+0x293/0x3b0
         ? __pfx_worker_thread+0x10/0x10
         kthread+0xec/0x1e0
         ? __pfx_kthread+0x10/0x10
         ? __pfx_kthread+0x10/0x10
         ret_from_fork+0x2f/0x50
         ? __pfx_kthread+0x10/0x10
         ret_from_fork_asm+0x1a/0x30
         </TASK>
        ---[ end trace 0000000000000000 ]---

    Fixes: 9e2ee5c ("net, bonding: Add XDP support to the bonding driver")
    Signed-off-by: Wang Liang <wangliang74@huawei.com>
    Acked-by: Jussi Maki <joamaki@gmail.com>
    Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
    Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
    Link: https://patch.msgid.link/20250321044852.1086551-1-wangliang74@huawei.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Signed-off-by: Hangbin Liu <haliu@redhat.com>
github-actions bot pushed a commit that referenced this pull request Nov 3, 2025
@gregkh
bonding: check xdp prog when set bond mode
6f3af80 
[ Upstream commit 094ee60 ]

Following operations can trigger a warning[1]:

    ip netns add ns1
    ip netns exec ns1 ip link add bond0 type bond mode balance-rr
    ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
    ip netns exec ns1 ip link set bond0 type bond mode broadcast
    ip netns del ns1

When delete the namespace, dev_xdp_uninstall() is called to remove xdp
program on bond dev, and bond_xdp_set() will check the bond mode. If bond
mode is changed after attaching xdp program, the warning may occur.

Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
with xdp program attached is not good. Add check for xdp program when set
bond mode.

    [1]
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
    Modules linked in:
    CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
    Workqueue: netns cleanup_net
    RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
    Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
    RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
    RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
    RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
    RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
    R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
    R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
    FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
    Call Trace:
     <TASK>
     ? __warn+0x83/0x130
     ? unregister_netdevice_many_notify+0x8d9/0x930
     ? report_bug+0x18e/0x1a0
     ? handle_bug+0x54/0x90
     ? exc_invalid_op+0x18/0x70
     ? asm_exc_invalid_op+0x1a/0x20
     ? unregister_netdevice_many_notify+0x8d9/0x930
     ? bond_net_exit_batch_rtnl+0x5c/0x90
     cleanup_net+0x237/0x3d0
     process_one_work+0x163/0x390
     worker_thread+0x293/0x3b0
     ? __pfx_worker_thread+0x10/0x10
     kthread+0xec/0x1e0
     ? __pfx_kthread+0x10/0x10
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x2f/0x50
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>
    ---[ end trace 0000000000000000 ]---

Fixes: 9e2ee5c ("net, bonding: Add XDP support to the bonding driver")
Signed-off-by: Wang Liang <wangliang74@huawei.com>
Acked-by: Jussi Maki <joamaki@gmail.com>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://patch.msgid.link/20250321044852.1086551-1-wangliang74@huawei.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Rajani Kantha <681739313@139.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
github-actions bot pushed a commit that referenced this pull request Nov 21, 2025
@igsilya @kuba-moo
net: openvswitch: remove never-working support for setting nsh fields
dfe28c4 
The validation of the set(nsh(...)) action is completely wrong.
It runs through the nsh_key_put_from_nlattr() function that is the
same function that validates NSH keys for the flow match and the
push_nsh() action.  However, the set(nsh(...)) has a very different
memory layout.  Nested attributes in there are doubled in size in
case of the masked set().  That makes proper validation impossible.

There is also confusion in the code between the 'masked' flag, that
says that the nested attributes are doubled in size containing both
the value and the mask, and the 'is_mask' that says that the value
we're parsing is the mask.  This is causing kernel crash on trying to
write into mask part of the match with SW_FLOW_KEY_PUT() during
validation, while validate_nsh() doesn't allocate any memory for it:

  BUG: kernel NULL pointer dereference, address: 0000000000000018
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 1c2383067 P4D 1c2383067 PUD 20b703067 PMD 0
  Oops: Oops: 0000 [#1] SMP NOPTI
  CPU: 8 UID: 0 Kdump: loaded Not tainted 6.17.0-rc4+ #107 PREEMPT(voluntary)
  RIP: 0010:nsh_key_put_from_nlattr+0x19d/0x610 [openvswitch]
  Call Trace:
   <TASK>
   validate_nsh+0x60/0x90 [openvswitch]
   validate_set.constprop.0+0x270/0x3c0 [openvswitch]
   __ovs_nla_copy_actions+0x477/0x860 [openvswitch]
   ovs_nla_copy_actions+0x8d/0x100 [openvswitch]
   ovs_packet_cmd_execute+0x1cc/0x310 [openvswitch]
   genl_family_rcv_msg_doit+0xdb/0x130
   genl_family_rcv_msg+0x14b/0x220
   genl_rcv_msg+0x47/0xa0
   netlink_rcv_skb+0x53/0x100
   genl_rcv+0x24/0x40
   netlink_unicast+0x280/0x3b0
   netlink_sendmsg+0x1f7/0x430
   ____sys_sendmsg+0x36b/0x3a0
   ___sys_sendmsg+0x87/0xd0
   __sys_sendmsg+0x6d/0xd0
   do_syscall_64+0x7b/0x2c0
   entry_SYSCALL_64_after_hwframe+0x76/0x7e

The third issue with this process is that while trying to convert
the non-masked set into masked one, validate_set() copies and doubles
the size of the OVS_KEY_ATTR_NSH as if it didn't have any nested
attributes.  It should be copying each nested attribute and doubling
them in size independently.  And the process must be properly reversed
during the conversion back from masked to a non-masked variant during
the flow dump.

In the end, the only two outcomes of trying to use this action are
either validation failure or a kernel crash.  And if somehow someone
manages to install a flow with such an action, it will most definitely
not do what it is supposed to, since all the keys and the masks are
mixed up.

Fixing all the issues is a complex task as it requires re-writing
most of the validation code.

Given that and the fact that this functionality never worked since
introduction, let's just remove it altogether.  It's better to
re-introduce it later with a proper implementation instead of trying
to fix it in stable releases.

Fixes: b2d0f5d ("openvswitch: enable NSH support")
Reported-by: Junvy Yang <zhuque@tencent.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Link: https://patch.msgid.link/20251112112246.95064-1-i.maximets@ovn.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
github-actions bot pushed a commit that referenced this pull request Dec 2, 2025
@igsilya @gregkh
net: openvswitch: remove never-working support for setting nsh fields
9c61d8f 
[ Upstream commit dfe28c4 ]

The validation of the set(nsh(...)) action is completely wrong.
It runs through the nsh_key_put_from_nlattr() function that is the
same function that validates NSH keys for the flow match and the
push_nsh() action.  However, the set(nsh(...)) has a very different
memory layout.  Nested attributes in there are doubled in size in
case of the masked set().  That makes proper validation impossible.

There is also confusion in the code between the 'masked' flag, that
says that the nested attributes are doubled in size containing both
the value and the mask, and the 'is_mask' that says that the value
we're parsing is the mask.  This is causing kernel crash on trying to
write into mask part of the match with SW_FLOW_KEY_PUT() during
validation, while validate_nsh() doesn't allocate any memory for it:

  BUG: kernel NULL pointer dereference, address: 0000000000000018
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 1c2383067 P4D 1c2383067 PUD 20b703067 PMD 0
  Oops: Oops: 0000 [#1] SMP NOPTI
  CPU: 8 UID: 0 Kdump: loaded Not tainted 6.17.0-rc4+ #107 PREEMPT(voluntary)
  RIP: 0010:nsh_key_put_from_nlattr+0x19d/0x610 [openvswitch]
  Call Trace:
   <TASK>
   validate_nsh+0x60/0x90 [openvswitch]
   validate_set.constprop.0+0x270/0x3c0 [openvswitch]
   __ovs_nla_copy_actions+0x477/0x860 [openvswitch]
   ovs_nla_copy_actions+0x8d/0x100 [openvswitch]
   ovs_packet_cmd_execute+0x1cc/0x310 [openvswitch]
   genl_family_rcv_msg_doit+0xdb/0x130
   genl_family_rcv_msg+0x14b/0x220
   genl_rcv_msg+0x47/0xa0
   netlink_rcv_skb+0x53/0x100
   genl_rcv+0x24/0x40
   netlink_unicast+0x280/0x3b0
   netlink_sendmsg+0x1f7/0x430
   ____sys_sendmsg+0x36b/0x3a0
   ___sys_sendmsg+0x87/0xd0
   __sys_sendmsg+0x6d/0xd0
   do_syscall_64+0x7b/0x2c0
   entry_SYSCALL_64_after_hwframe+0x76/0x7e

The third issue with this process is that while trying to convert
the non-masked set into masked one, validate_set() copies and doubles
the size of the OVS_KEY_ATTR_NSH as if it didn't have any nested
attributes.  It should be copying each nested attribute and doubling
them in size independently.  And the process must be properly reversed
during the conversion back from masked to a non-masked variant during
the flow dump.

In the end, the only two outcomes of trying to use this action are
either validation failure or a kernel crash.  And if somehow someone
manages to install a flow with such an action, it will most definitely
not do what it is supposed to, since all the keys and the masks are
mixed up.

Fixing all the issues is a complex task as it requires re-writing
most of the validation code.

Given that and the fact that this functionality never worked since
introduction, let's just remove it altogether.  It's better to
re-introduce it later with a proper implementation instead of trying
to fix it in stable releases.

Fixes: b2d0f5d ("openvswitch: enable NSH support")
Reported-by: Junvy Yang <zhuque@tencent.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Link: https://patch.msgid.link/20251112112246.95064-1-i.maximets@ovn.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PlaidCat PlaidCat PlaidCat approved these changes

@jason-rodri jason-rodri jason-rodri approved these changes

+1 more reviewer

@gvrose8192 gvrose8192 gvrose8192 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bmastbergen @PlaidCat @gvrose8192 @jason-rodri