Skip to content
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

Microphones the LEAP way #151

Draft
wants to merge 532 commits into
base: asahi-wip
Choose a base branch
from
Draft

Microphones the LEAP way #151

wants to merge 532 commits into from

Conversation

povik
Copy link
Member

@povik povik commented May 26, 2023 •
edited
Loading

This has few small drivers and a macaudio extension to bring up internal microphones.

It depends on AsahiLinux/m1n1#314 and requires firmware at runtime (downloadable from here). All of the microphone-equipped Apple Silicon machines seem to have a 3-way microphone array. So far, the firmware picks out a single channel selectable by an alsamixer knob.

In my testing on j456 the first two selectable mic inputs were broken, not sure what's the cause there (it's something subtle, the mics should be connected the same way as on j314). On j314 all of the inputs were working.

Model Support

Only for j314 and j456 have all the DT nodes been added, though what's missing for other models with the same SoC (t8103 or t6000) should be a simple copy-paste of the node below /sound and the near-by dmic node. The SoC nodes were also almost added for t8112 and t6020, but there's few bits of missing information that needs to be found out by a quick experiment.

This has all of the AOP power domains, which have the no_ps flag in ADT, added to the DT. This is primarily to check that powering them all off doesn't blow up. I expect (if we indeed go through with bypassing AOP) that we will eventually only describe the few AOP power domains of interest, and power off all of the remainder in the bootloader. For non-t8103 targets we don't even have a reliable description of the addresses of the AOP PD registers in ADT, though they can be estimated and we have at least a reliable range in the AOP DAPF.

One More Thing

I let some commits piggyback off this PR, the first three commits are unrelated fixups which I think we want for asahi-wip.

jannau and others added 30 commits April 24, 2023 19:53
@jannau @marcan
drm/apple: Drop unsupported DRM_FORMAT_ARGB2101010
a8a5aa7 
Depends on https://gitlab.freedesktop.org/asahi/mesa/-/merge_requests/5

Signed-off-by: Janne Grunau <[email protected]>
@marcan
dcp: Allow unused trampolines
bec9b9c 
Signed-off-by: Hector Martin <[email protected]>
@marcan
dcp: Add get_tiling_state
4cd47de 
Signed-off-by: Hector Martin <[email protected]>
@marcan
dcp: 42-bit DMA masks
210457a 
Signed-off-by: Hector Martin <[email protected]>
@marcan
dcp: T602X bwreq support
613ee7e 
Signed-off-by: Hector Martin <[email protected]>
@marcan
dcp: Warn if DMA mapping fails
9a925bc 
Signed-off-by: Hector Martin <[email protected]>
@ojeda @marcan
arm64: rust: Enable Rust support for AArch64
ad6c983 
This commit provides the build flags for Rust for AArch64. The core Rust
support already in the kernel does the rest.

The Rust samples have been tested with this commit.

[jcunliffe: Arm specific parts taken from Miguel's upstream tree]

Signed-off-by: Miguel Ojeda <[email protected]>
Co-developed-by: Jamie Cunliffe <[email protected]>
Signed-off-by: Jamie Cunliffe <[email protected]>
Signed-off-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
@JamieCunliffe @marcan
arm64: rust: Enable PAC support for Rust.
ead1a45 
Enable the PAC ret and BTI options in the Rust build flags to match
the options that are used when building C.

Signed-off-by: Jamie Cunliffe <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
@JamieCunliffe @marcan
arm64: rust: Disable neon and fp target features.
4958626 
Disable the neon and fp target features to avoid fp & simd
registers. The use of fp-armv8 will cause a warning from rustc about
an unknown feature that is specified. The target feature is still
passed through to LLVM, this behaviour is documented as part of the
warning. This will be fixed in a future version of the rustc
toolchain.

Signed-off-by: Jamie Cunliffe <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
@fbq @marcan
rust: sync: impl {Debug,Display} for {Unique,}Arc
0e74f05 
This allows printing the inner data of `Arc` and its friends if the
inner data implements `Display` or `Debug`. It's useful for logging and
debugging purpose.

Signed-off-by: Boqun Feng <[email protected]>
Reviwed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Andreas Hindborg <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
@fbq @marcan
sample: rust: print: Add sampe code for Arc printing
8cd6e03 
This both demonstrates the usage of different print format in Rust and
serves as a selftest for the `Display` and `Debug` implementation of
`Arc` and its friends.

Signed-off-by: Boqun Feng <[email protected]>
Reviewed-by: Björn Roy Baron <[email protected]>
Reviewed-by: Finn Behrens <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Andreas Hindborg <[email protected]>
@asahilina @marcan
soc: apple: rtkit: Fix buffer address field width
f806499 
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: time: New module for timekeeping functions
e7074a3 
This module is intended to contain functions related to kernel
timekeeping and time. Initially, this just wraps ktime_get() and
ktime_get_boottime() and returns them as core::time::Duration instances.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: macros: Make expect_punct() return the Punct directly
1f3243e 
This makes it mirror the way expect_ident() works, and means we can more
easily push the result back into the token stream.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: concat_idents: Allow :: paths in first argument
7d24b62 
This makes things like concat_idents!(bindings::foo, bar) work.
Otherwise, there is no way to concatenate two idents and then use the
result as part of a type path.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: macros: Allow specifying multiple module aliases
60abc1a 
Modules can (and usually do) have multiple alias tags, in order to
specify multiple possible device matches for autoloading. Allow this by
changing the alias ModuleInfo field to an Option<Vec<String>>.

Note: For normal device IDs this is autogenerated by modpost (which is
not properly integrated with Rust support yet), so it is useful to be
able to manually add device match aliases for now, and should still be
useful in the future for corner cases that modpost does not handle.

This pulls in the expect_group() helper from the rfl/rust branch
(with credit to authors).

Co-developed-by: Miguel Ojeda <[email protected]>
Co-developed-by: Finn Behrens <[email protected]>
Co-developed-by: Sumera Priyadarsini <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: ioctl: Add ioctl number manipulation functions
8256c64 
Add simple 1:1 wrappers of the C ioctl number manipulation functions.
Since these are macros we cannot bindgen them directly, and since they
should be usable in const context we cannot use helper wrappers, so
we'll have to reimplement them in Rust. Thankfully, the C headers do
declare defines for the relevant bitfield positions, so we don't need
to duplicate that.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: sync: arc: implement Arc<dyn Any + Send + Sync>::downcast()
eaaa6b6 
This mirrors the standard library's alloc::sync::Arc::downcast().

Based on the Rust standard library implementation, ver 1.62.0,
licensed under "Apache-2.0 OR MIT", from:

    https://github.com/rust-lang/rust/tree/1.62.0/library/alloc/src

For copyright details, please see:

        https://github.com/rust-lang/rust/blob/1.62.0/COPYRIGHT

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: Enable feature new_uninit for the kernel crate
abe21fe 
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: sync: arc: Add UniqueArc<MaybeUninit<T>::assume_init()
00c0d4b 
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: error: Add Error::to_ptr()
350e759 
This is the Rust equivalent to ERR_PTR(), for use in C callbacks.
Marked as #[allow(dead_code)] for now, since it does not have any
consumers yet.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: error: Add Error::from_kernel_errno()
f279115 
Add a function to create `Error` values out of a kernel error return,
which safely upholds the invariant that the error code is well-formed
(negative and greater than -MAX_ERRNO). If a malformed code is passed
in, it will be converted to EINVAL.

Imported from rust-for-linux/rust as authored by Miguel and Fox with
refactoring from Wedson.

Co-developed-by: Miguel Ojeda <[email protected]>
Co-developed-by: Fox Chen <[email protected]>
Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Fox Chen <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: error: Add to_result() helper
4cb6485 
Add a to_result() helper to convert kernel C return values to a Rust
Result, mapping >=0 values to Ok(()) and negative values to Err(...),
with Error::from_kernel_errno() ensuring that the errno is within range.

Imported from rust-for-linux/rust, originally developed by Wedson as part of the
AMBA device driver support.

Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@marcan
rust: error: Add a helper to convert a C ERR_PTR to a Result
4bbbf79 
Some kernel C API functions return a pointer which embeds an optional
`errno`. Callers are supposed to check the returned pointer with
`IS_ERR()` and if this returns `true`, retrieve the `errno` using
`PTR_ERR()`.

Create a Rust helper function to implement the Rust equivalent:
transform a `*mut T` to `Result<*mut T>`.

Lina: Imported from rust-for-linux/linux, with subsequent refactoring
and contributions squashed in and attributed below. Replaced usage of
from_kernel_errno_unchecked() with an open-coded constructor, since this
is the only user anyway.

Co-developed-by: Boqun Feng <[email protected]>
Signed-off-by: Boqun Feng <[email protected]>
Co-developed-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Co-developed-by: Fox Chen <[email protected]>
Signed-off-by: Fox Chen <[email protected]>
Co-developed-by: Gary Guo <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
Signed-off-by: Sven Van Asbroeck <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: error: Add from_kernel_result!() macro
1214c47 
Add a helper macro to easily return C result codes from a Rust function
that calls functions which return a Result<T>.

Imported from rust-for-linux/rust, originally developed by Wedson as
part of file_operations.rs.

Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Co-developed-by: Fox Chen <[email protected]>
Signed-off-by: Fox Chen <[email protected]>
Co-developed-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: Import upstream alloc::vec::set_len_on_drop module
4f1046a 
This is a subset of the Rust standard library `alloc` crate,
version 1.66.0, licensed under "Apache-2.0 OR MIT", from:

    https://github.com/rust-lang/rust/tree/1.66.0/library/alloc/src

The file is copied as-is, with no modifications whatsoever
(not even adding the SPDX identifiers).

For copyright details, please see:

    https://github.com/rust-lang/rust/blob/1.66.0/COPYRIGHT

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: Import upstream alloc::vec::spec_extend module
e9eb039 
This is a subset of the Rust standard library `alloc` crate,
version 1.66.0, licensed under "Apache-2.0 OR MIT", from:

    https://github.com/rust-lang/rust/tree/1.66.0/library/alloc/src

The file is copied as-is, with no modifications whatsoever
(not even adding the SPDX identifiers).

For copyright details, please see:

    https://github.com/rust-lang/rust/blob/1.66.0/COPYRIGHT

Signed-off-by: Asahi Lina <[email protected]>
@ojeda @marcan
rust: alloc: vec: Add some try_* methods we need
e0f409b 
Add some missing fallible methods that we need.

They are all marked as:

    #[stable(feature = "kernel", since = "1.0.0")]

for easy identification.

Lina: Extracted from 487d757 ("rust: alloc: add some `try_*`
methods we need") in rust-for-linux/rust.

Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: xarray: Add an abstraction for XArray
76306a4 
The XArray is an abstract data type which behaves like a very large
array of pointers. Add a Rust abstraction for this data type.

The initial implementation uses explicit locking on get operations and
returns a guard which blocks mutation, ensuring that the referenced
object remains alive. To avoid excessive serialization, users are
expected to use an inner type that can be efficiently cloned (such as
Arc<T>), and eagerly clone and drop the guard to unblock other users
after a lookup.

Future variants may support using RCU instead to avoid mutex locking.

This abstraction also introduces a reservation mechanism, which can be
used by alloc-capable XArrays to reserve a free slot without immediately
filling it, and then do so at a later time. If the reservation is
dropped without being filled, the slot is freed again for other users,
which eliminates the need for explicit cleanup code.

Signed-off-by: Asahi Lina <[email protected]>
@asahilina @marcan
rust: Add a Sealed trait
7b883dc 
Some traits exposed by the kernel crate may not be intended to be
implemented by downstream modules. Add a Sealed trait to allow avoiding
this using the sealed trait pattern.

Signed-off-by: Asahi Lina <[email protected]>
@jannau jannau force-pushed the asahi-wip branch 3 times, most recently from 138f32e to 98953ab Compare June 28, 2024 06:39
@jannau jannau force-pushed the asahi-wip branch 2 times, most recently from f0760b4 to db6542f Compare July 11, 2024 19:42
@jannau jannau force-pushed the asahi-wip branch 4 times, most recently from 83a2784 to 15b881f Compare July 29, 2024 19:30
@jannau jannau force-pushed the asahi-wip branch 2 times, most recently from ccc12d2 to 24c1838 Compare August 21, 2024 20:21
jannau pushed a commit that referenced this pull request Aug 31, 2024
@mairacanal @gregkh
drm/v3d: Fix out-of-bounds read in v3d_csd_job_run()
d656b82 
[ Upstream commit 497d370 ]

When enabling UBSAN on Raspberry Pi 5, we get the following warning:

[  387.894977] UBSAN: array-index-out-of-bounds in drivers/gpu/drm/v3d/v3d_sched.c:320:3
[  387.903868] index 7 is out of range for type '__u32 [7]'
[  387.909692] CPU: 0 PID: 1207 Comm: kworker/u16:2 Tainted: G        WC         6.10.3-v8-16k-numa #151
[  387.919166] Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT)
[  387.925961] Workqueue: v3d_csd drm_sched_run_job_work [gpu_sched]
[  387.932525] Call trace:
[  387.935296]  dump_backtrace+0x170/0x1b8
[  387.939403]  show_stack+0x20/0x38
[  387.942907]  dump_stack_lvl+0x90/0xd0
[  387.946785]  dump_stack+0x18/0x28
[  387.950301]  __ubsan_handle_out_of_bounds+0x98/0xd0
[  387.955383]  v3d_csd_job_run+0x3a8/0x438 [v3d]
[  387.960707]  drm_sched_run_job_work+0x520/0x6d0 [gpu_sched]
[  387.966862]  process_one_work+0x62c/0xb48
[  387.971296]  worker_thread+0x468/0x5b0
[  387.975317]  kthread+0x1c4/0x1e0
[  387.978818]  ret_from_fork+0x10/0x20
[  387.983014] ---[ end trace ]---

This happens because the UAPI provides only seven configuration
registers and we are reading the eighth position of this u32 array.

Therefore, fix the out-of-bounds read in `v3d_csd_job_run()` by
accessing only seven positions on the '__u32 [7]' array. The eighth
register exists indeed on V3D 7.1, but it isn't currently used. That
being so, let's guarantee that it remains unused and add a note that it
could be set in a future patch.

Fixes: 0ad5bc1 ("drm/v3d: fix up register addresses for V3D 7.x")
Reported-by: Tvrtko Ursulin <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
Reviewed-by: Iago Toral Quiroga <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
jannau pushed a commit that referenced this pull request Sep 16, 2024
@mairacanal
drm/v3d: Fix out-of-bounds read in v3d_csd_job_run()
497d370 
When enabling UBSAN on Raspberry Pi 5, we get the following warning:

[  387.894977] UBSAN: array-index-out-of-bounds in drivers/gpu/drm/v3d/v3d_sched.c:320:3
[  387.903868] index 7 is out of range for type '__u32 [7]'
[  387.909692] CPU: 0 PID: 1207 Comm: kworker/u16:2 Tainted: G        WC         6.10.3-v8-16k-numa #151
[  387.919166] Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT)
[  387.925961] Workqueue: v3d_csd drm_sched_run_job_work [gpu_sched]
[  387.932525] Call trace:
[  387.935296]  dump_backtrace+0x170/0x1b8
[  387.939403]  show_stack+0x20/0x38
[  387.942907]  dump_stack_lvl+0x90/0xd0
[  387.946785]  dump_stack+0x18/0x28
[  387.950301]  __ubsan_handle_out_of_bounds+0x98/0xd0
[  387.955383]  v3d_csd_job_run+0x3a8/0x438 [v3d]
[  387.960707]  drm_sched_run_job_work+0x520/0x6d0 [gpu_sched]
[  387.966862]  process_one_work+0x62c/0xb48
[  387.971296]  worker_thread+0x468/0x5b0
[  387.975317]  kthread+0x1c4/0x1e0
[  387.978818]  ret_from_fork+0x10/0x20
[  387.983014] ---[ end trace ]---

This happens because the UAPI provides only seven configuration
registers and we are reading the eighth position of this u32 array.

Therefore, fix the out-of-bounds read in `v3d_csd_job_run()` by
accessing only seven positions on the '__u32 [7]' array. The eighth
register exists indeed on V3D 7.1, but it isn't currently used. That
being so, let's guarantee that it remains unused and add a note that it
could be set in a future patch.

Fixes: 0ad5bc1 ("drm/v3d: fix up register addresses for V3D 7.x")
Reported-by: Tvrtko Ursulin <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
Reviewed-by: Iago Toral Quiroga <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
@jannau jannau force-pushed the asahi-wip branch 2 times, most recently from 4fccab3 to 92d5e5b Compare September 21, 2024 13:15
@jannau jannau force-pushed the asahi-wip branch 2 times, most recently from addb584 to 3a0ab10 Compare October 6, 2024 18:07
@allmazz
Copy link

allmazz commented Oct 21, 2024

any updates?

jannau pushed a commit that referenced this pull request Nov 9, 2024
@idosch @gregkh
mlxsw: spectrum_ipip: Fix memory leak when changing remote IPv6 address
c1bbdbe 
[ Upstream commit 12ae97c ]

The device stores IPv6 addresses that are used for encapsulation in
linear memory that is managed by the driver.

Changing the remote address of an ip6gre net device never worked
properly, but since cited commit the following reproducer [1] would
result in a warning [2] and a memory leak [3]. The problem is that the
new remote address is never added by the driver to its hash table (and
therefore the device) and the old address is never removed from it.

Fix by programming the new address when the configuration of the ip6gre
net device changes and removing the old one. If the address did not
change, then the above would result in increasing the reference count of
the address and then decreasing it.

[1]
 # ip link add name bla up type ip6gre local 2001:db8:1::1 remote 2001:db8:2::1 tos inherit ttl inherit
 # ip link set dev bla type ip6gre remote 2001:db8:3::1
 # ip link del dev bla
 # devlink dev reload pci/0000:01:00.0

[2]
WARNING: CPU: 0 PID: 1682 at drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3002 mlxsw_sp_ipv6_addr_put+0x140/0x1d0
Modules linked in:
CPU: 0 UID: 0 PID: 1682 Comm: ip Not tainted 6.12.0-rc3-custom-g86b5b55bc835 #151
Hardware name: Nvidia SN5600/VMOD0013, BIOS 5.13 05/31/2023
RIP: 0010:mlxsw_sp_ipv6_addr_put+0x140/0x1d0
[...]
Call Trace:
 <TASK>
 mlxsw_sp_router_netdevice_event+0x55f/0x1240
 notifier_call_chain+0x5a/0xd0
 call_netdevice_notifiers_info+0x39/0x90
 unregister_netdevice_many_notify+0x63e/0x9d0
 rtnl_dellink+0x16b/0x3a0
 rtnetlink_rcv_msg+0x142/0x3f0
 netlink_rcv_skb+0x50/0x100
 netlink_unicast+0x242/0x390
 netlink_sendmsg+0x1de/0x420
 ____sys_sendmsg+0x2bd/0x320
 ___sys_sendmsg+0x9a/0xe0
 __sys_sendmsg+0x7a/0xd0
 do_syscall_64+0x9e/0x1a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

[3]
unreferenced object 0xffff898081f597a0 (size 32):
  comm "ip", pid 1626, jiffies 4294719324
  hex dump (first 32 bytes):
    20 01 0d b8 00 02 00 00 00 00 00 00 00 00 00 01   ...............
    21 49 61 83 80 89 ff ff 00 00 00 00 01 00 00 00  !Ia.............
  backtrace (crc fd9be911):
    [<00000000df89c55d>] __kmalloc_cache_noprof+0x1da/0x260
    [<00000000ff2a1ddb>] mlxsw_sp_ipv6_addr_kvdl_index_get+0x281/0x340
    [<000000009ddd445d>] mlxsw_sp_router_netdevice_event+0x47b/0x1240
    [<00000000743e7757>] notifier_call_chain+0x5a/0xd0
    [<000000007c7b9e13>] call_netdevice_notifiers_info+0x39/0x90
    [<000000002509645d>] register_netdevice+0x5f7/0x7a0
    [<00000000c2e7d2a9>] ip6gre_newlink_common.isra.0+0x65/0x130
    [<0000000087cd6d8d>] ip6gre_newlink+0x72/0x120
    [<000000004df7c7cc>] rtnl_newlink+0x471/0xa20
    [<0000000057ed632a>] rtnetlink_rcv_msg+0x142/0x3f0
    [<0000000032e0d5b5>] netlink_rcv_skb+0x50/0x100
    [<00000000908bca63>] netlink_unicast+0x242/0x390
    [<00000000cdbe1c87>] netlink_sendmsg+0x1de/0x420
    [<0000000011db153e>] ____sys_sendmsg+0x2bd/0x320
    [<000000003b6d53eb>] ___sys_sendmsg+0x9a/0xe0
    [<00000000cae27c62>] __sys_sendmsg+0x7a/0xd0

Fixes: cf42911 ("mlxsw: spectrum_ipip: Use common hash table for IPv6 address mapping")
Reported-by: Maksym Yaremchuk <[email protected]>
Signed-off-by: Ido Schimmel <[email protected]>
Reviewed-by: Petr Machata <[email protected]>
Signed-off-by: Petr Machata <[email protected]>
Link: https://patch.msgid.link/e91012edc5a6cb9df37b78fd377f669381facfcb.1729866134.git.petrm@nvidia.com
Signed-off-by: Jakub Kicinski <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
@WhatAmISupposedToPutHere WhatAmISupposedToPutHere mentioned this pull request Nov 9, 2024
Draft
@jannau jannau force-pushed the asahi-wip branch 2 times, most recently from 1543478 to 44dba3b Compare November 17, 2024 18:06
jannau pushed a commit that referenced this pull request Nov 17, 2024
@idosch @kuba-moo
mlxsw: spectrum_ipip: Fix memory leak when changing remote IPv6 address
12ae97c 
The device stores IPv6 addresses that are used for encapsulation in
linear memory that is managed by the driver.

Changing the remote address of an ip6gre net device never worked
properly, but since cited commit the following reproducer [1] would
result in a warning [2] and a memory leak [3]. The problem is that the
new remote address is never added by the driver to its hash table (and
therefore the device) and the old address is never removed from it.

Fix by programming the new address when the configuration of the ip6gre
net device changes and removing the old one. If the address did not
change, then the above would result in increasing the reference count of
the address and then decreasing it.

[1]
 # ip link add name bla up type ip6gre local 2001:db8:1::1 remote 2001:db8:2::1 tos inherit ttl inherit
 # ip link set dev bla type ip6gre remote 2001:db8:3::1
 # ip link del dev bla
 # devlink dev reload pci/0000:01:00.0

[2]
WARNING: CPU: 0 PID: 1682 at drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3002 mlxsw_sp_ipv6_addr_put+0x140/0x1d0
Modules linked in:
CPU: 0 UID: 0 PID: 1682 Comm: ip Not tainted 6.12.0-rc3-custom-g86b5b55bc835 #151
Hardware name: Nvidia SN5600/VMOD0013, BIOS 5.13 05/31/2023
RIP: 0010:mlxsw_sp_ipv6_addr_put+0x140/0x1d0
[...]
Call Trace:
 <TASK>
 mlxsw_sp_router_netdevice_event+0x55f/0x1240
 notifier_call_chain+0x5a/0xd0
 call_netdevice_notifiers_info+0x39/0x90
 unregister_netdevice_many_notify+0x63e/0x9d0
 rtnl_dellink+0x16b/0x3a0
 rtnetlink_rcv_msg+0x142/0x3f0
 netlink_rcv_skb+0x50/0x100
 netlink_unicast+0x242/0x390
 netlink_sendmsg+0x1de/0x420
 ____sys_sendmsg+0x2bd/0x320
 ___sys_sendmsg+0x9a/0xe0
 __sys_sendmsg+0x7a/0xd0
 do_syscall_64+0x9e/0x1a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

[3]
unreferenced object 0xffff898081f597a0 (size 32):
  comm "ip", pid 1626, jiffies 4294719324
  hex dump (first 32 bytes):
    20 01 0d b8 00 02 00 00 00 00 00 00 00 00 00 01   ...............
    21 49 61 83 80 89 ff ff 00 00 00 00 01 00 00 00  !Ia.............
  backtrace (crc fd9be911):
    [<00000000df89c55d>] __kmalloc_cache_noprof+0x1da/0x260
    [<00000000ff2a1ddb>] mlxsw_sp_ipv6_addr_kvdl_index_get+0x281/0x340
    [<000000009ddd445d>] mlxsw_sp_router_netdevice_event+0x47b/0x1240
    [<00000000743e7757>] notifier_call_chain+0x5a/0xd0
    [<000000007c7b9e13>] call_netdevice_notifiers_info+0x39/0x90
    [<000000002509645d>] register_netdevice+0x5f7/0x7a0
    [<00000000c2e7d2a9>] ip6gre_newlink_common.isra.0+0x65/0x130
    [<0000000087cd6d8d>] ip6gre_newlink+0x72/0x120
    [<000000004df7c7cc>] rtnl_newlink+0x471/0xa20
    [<0000000057ed632a>] rtnetlink_rcv_msg+0x142/0x3f0
    [<0000000032e0d5b5>] netlink_rcv_skb+0x50/0x100
    [<00000000908bca63>] netlink_unicast+0x242/0x390
    [<00000000cdbe1c87>] netlink_sendmsg+0x1de/0x420
    [<0000000011db153e>] ____sys_sendmsg+0x2bd/0x320
    [<000000003b6d53eb>] ___sys_sendmsg+0x9a/0xe0
    [<00000000cae27c62>] __sys_sendmsg+0x7a/0xd0

Fixes: cf42911 ("mlxsw: spectrum_ipip: Use common hash table for IPv6 address mapping")
Reported-by: Maksym Yaremchuk <[email protected]>
Signed-off-by: Ido Schimmel <[email protected]>
Reviewed-by: Petr Machata <[email protected]>
Signed-off-by: Petr Machata <[email protected]>
Link: https://patch.msgid.link/e91012edc5a6cb9df37b78fd377f669381facfcb.1729866134.git.petrm@nvidia.com
Signed-off-by: Jakub Kicinski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.