rose: guard rose_transmit_link() against a NULL neighbour - #5
Merged
kuba-moo merged 1 commit intoSep 11, 2026
Merged
Conversation
rose_kick() and rose_write_internal() call rose_transmit_link(skb, rose->neighbour) without checking that rose->neighbour is still set. Since commit e8eb0c6 ("rose: clear neighbour pointer after rose_neigh_put() in state machines"), the state machines routinely set rose->neighbour to NULL once the underlying AX.25 link is gone, while leaving the socket in ROSE_STATE_3. rose_kick() only checks the state, not the neighbour, so a write() on such a socket reaches rose_transmit_link() with neigh == NULL, which immediately dereferences neigh->loopback and crashes: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000036 ... pc : rose_transmit_link+0x14/0x1c8 [rose] lr : rose_kick+0xec/0x198 [rose] Call trace: rose_transmit_link+0x14/0x1c8 [rose] (P) rose_kick+0xec/0x198 [rose] rose_sendmsg+0x260/0x3c8 [rose] Reproduced on f6bvp-8 (a real ROSE/FPAC node) when the BBS daemon wrote to a socket whose neighbour had just been cleared. Confirmed by recompiling rose_link.o with a static_assert on offsetof(struct rose_neigh, loopback), which matches the faulting address (0x36) exactly. Drop the skb and return early when neigh is NULL, matching what rose_transmit_link() already does for other frames it cannot send. Fixes: e8eb0c6 ("rose: clear neighbour pointer after rose_neigh_put() in state machines") Signed-off-by: Bernard Pidoux <bernard.f6bvp@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rose_kick() and rose_write_internal() call rose_transmit_link(skb,
rose->neighbour) without checking that rose->neighbour is still set.
Since commit e8eb0c6 ("rose: clear neighbour pointer after
rose_neigh_put() in state machines"), the state machines routinely set
rose->neighbour to NULL once the underlying AX.25 link is gone, while
leaving the socket in ROSE_STATE_3. rose_kick() only checks the state,
not the neighbour, so a write() on such a socket reaches
rose_transmit_link() with neigh == NULL, which immediately
dereferences neigh->loopback and crashes:
Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000036
...
pc : rose_transmit_link+0x14/0x1c8 [rose]
lr : rose_kick+0xec/0x198 [rose]
Call trace:
rose_transmit_link+0x14/0x1c8 [rose] (P)
rose_kick+0xec/0x198 [rose]
rose_sendmsg+0x260/0x3c8 [rose]
Reproduced on f6bvp-8 (a real ROSE/FPAC node) when the BBS daemon
wrote to a socket whose neighbour had just been cleared. Confirmed by
recompiling rose_link.o with a static_assert on
offsetof(struct rose_neigh, loopback), which matches the faulting
address (0x36) exactly.
Fix: drop the skb and return early when neigh is NULL, matching what
rose_transmit_link() already does for other frames it cannot send.
Fixes: e8eb0c6 ("rose: clear neighbour pointer after rose_neigh_put() in state machines")
Single commit, applies cleanly to current main.