Bug 232192 - Harmonize udp_input() and udp6_input() locking; should make the udp6 code simpler
Summary: Harmonize udp_input() and udp6_input() locking; should make the udp6 code sim...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Bjoern A. Zeeb
URL:
Keywords: feature
Depends on:
Blocks: 232348
  Show dependency treegraph
 
Reported: 2018-10-11 23:58 UTC by Bjoern A. Zeeb
Modified: 2021-01-12 14:41 UTC (History)
3 users (show)

See Also:


Attachments
screenshot from ipmi (836.73 KB, image/png)
2021-01-11 11:56 UTC, Andrey V. Elsukov
no flags Details
proposed patch (untested) (1.58 KB, patch)
2021-01-12 14:41 UTC, Andrey V. Elsukov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bjoern A. Zeeb freebsd_committer 2018-10-11 23:58:27 UTC
Check if we need to apply these two commits from udp6_input() also to IPv4
( after https://reviews.freebsd.org/D17525 ) is in.

https://svnweb.freebsd.org/base?view=revision&revision=335919
https://svnweb.freebsd.org/base?view=revision&revision=335958
Comment 1 Bjoern A. Zeeb freebsd_committer 2018-10-12 21:21:29 UTC
The locking is a lot simpler in legacy IP, so we don't have to apply the changes 1:1 but can get away with a single check.
Comment 2 Bjoern A. Zeeb freebsd_committer 2018-10-12 21:34:05 UTC
https://reviews.freebsd.org/D17540



XXX TODO: harmonize IPv6 and IPv4 locking and code flow.
Comment 3 commit-hook freebsd_committer 2018-10-12 22:52:25 UTC
A commit references this bug:

Author: bz
Date: Fri Oct 12 22:51:45 UTC 2018
New revision: 339339
URL: https://svnweb.freebsd.org/changeset/base/339339

Log:
  In udp_input() when walking the pcblist we can come across
  an inp marked FREED after the epoch(9) changes.
  Check once we hold the lock and skip the inp if it is the case.

  Contrary to IPv6 the locking of the inp is outside the multicast
  section and hence a single check seems to suffice.

  PR:		232192
  Reviewed by:	mmacy, markj
  Approved by:	re (kib)
  Differential Revision:	https://reviews.freebsd.org/D17540

Changes:
  head/sys/netinet/udp_usrreq.c
Comment 4 Andrey V. Elsukov freebsd_committer 2021-01-11 11:56:02 UTC
Created attachment 221454 [details]
screenshot from ipmi

Hi Bjoern,

it seems there is still possible NULL pointer dereference panic in the related code. PCB is currently protected by NET_EPOCH section, and we can safely make access to PCB while holding INP_RLOCK().
But access to inp->inp_socket is not safe without the lock. 
I attached screenshot of panic, unfortunately there is no core dump. From the kgdb I obtined the line number, where the panic occured:

 415                         if ((last->inp_socket->so_options &
 416                              (SO_REUSEPORT|SO_REUSEPORT_LB|SO_REUSEADDR)) == 0)
 417                                 break;
Comment 5 Andrey V. Elsukov freebsd_committer 2021-01-12 14:41:33 UTC
Created attachment 221493 [details]
proposed patch (untested)

Probably, this patch should fix the problem. I'll test it for some time, then report back.