Bug 130386 - [patch] add locking for generic interface address manipulations through ioctl path
Summary: [patch] add locking for generic interface address manipulations through ioctl...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.1-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Robert Watson
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2009-01-11 19:40 UTC by Eygene Ryabinkin
Modified: 2022-10-17 12:38 UTC (History)
0 users

See Also:


Attachments
Lock-network-interface-address-manipulations.diff (14.88 KB, patch)
2009-01-11 19:40 UTC, Eygene Ryabinkin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eygene Ryabinkin 2009-01-11 19:40:02 UTC
Manipulation of interface addresses inside generic ioctl() handler
in_control() (sys/netinet/in.c) was not locked, so races from multiple
sumiltaneous invocations of ioctl's ADD/DELETE actions could occur and
the results will vary from incosistent data to the kernel panics.

More verbose description is in the patch below.

Fix: The patch below fixes the things for me.  rik@ will try to test this
patch on his system, but he runs 5.x on the host in question, so he
needs some time to understand and possibly port the patch.
How-To-Repeat: 
In my very case, the kernel panic is easily triggered just by restart of
the dhclient on my ethernet interface: 'ifaddr' mutex gets destroyed and
then used for locking (sleeping).  Actually, the bug's reproducibility
will vary from machine to machine.

rik@ sees another strange effects on his PPPoE interfaces: sometimes the
address list has repeating address/mask pairs.  This could be also
related to this race when two ioctsl ADD actions are racing or
ADD/DELETE actions are racing differently from mine's case.
Comment 1 Robert Watson freebsd_committer freebsd_triage 2009-02-10 09:31:03 UTC
State Changed
From-To: open->analyzed

This is a known problem; I will review this patch but think we may 
want to modify it in a number of ways. 


Comment 2 Robert Watson freebsd_committer freebsd_triage 2009-02-10 09:31:03 UTC
Responsible Changed
From-To: freebsd-bugs->rwatson

Grab ownership.
Comment 3 Eygene Ryabinkin 2009-04-09 22:14:09 UTC
Robert, good day.

Tue, Feb 10, 2009 at 09:31:53AM +0000, rwatson@FreeBSD.org wrote:
> Synopsis: [patch] add locking for generic interface address
> manipulations through ioctl path
>
> State-Changed-By: rwatson
> State-Changed-When: Tue Feb 10 09:31:03 UTC 2009
> State-Changed-Why:
> This is a known problem; I will review this patch but think we may
> want to modify it in a number of ways.

Sorry to bother, but may I ask about the status of this PR?  May be you
can give me some hints about the ways you want to modify the patch and I
will be able produce the better variant?  Or may be there are some other
patches to this problem and I can learn something from them?

Thanks!
-- 
Eygene
 _                ___       _.--.   #
 \`.|\..----...-'`   `-._.-'_.-'`   #  Remember that it is hard
 /  ' `         ,       __.--'      #  to read the on-line manual
 )/' _/     \   `-_,   /            #  while single-stepping the kernel.
 `-'" `"\_  ,_.-;_.-\_ ',  fsc/as   #
     _.-'_./   {_.'   ; /           #    -- FreeBSD Developers handbook
    {_.-``-'         {_/            #
Comment 4 Robert Watson freebsd_committer freebsd_triage 2009-04-09 23:26:04 UTC
On Fri, 10 Apr 2009, Eygene Ryabinkin wrote:

> Robert, good day.
>
> Tue, Feb 10, 2009 at 09:31:53AM +0000, rwatson@FreeBSD.org wrote:
>> Synopsis: [patch] add locking for generic interface address
>> manipulations through ioctl path
>>
>> State-Changed-By: rwatson
>> State-Changed-When: Tue Feb 10 09:31:03 UTC 2009
>> State-Changed-Why:
>> This is a known problem; I will review this patch but think we may
>> want to modify it in a number of ways.
>
> Sorry to bother, but may I ask about the status of this PR?  May be you can 
> give me some hints about the ways you want to modify the patch and I will be 
> able produce the better variant?  Or may be there are some other patches to 
> this problem and I can learn something from them?

Hi Eygene:

I am working on this, slowly, but it's on the back-burner due to other stuff 
in the queue.  I needed to significantly expand address list locking in the 
work-in-progress, so my current target is to bring the changes in before 8.0, 
then look at an MFC perhaps in time for 7.3.  This work really relies on 
having rmlocks in 7.x, which provide a much lower acquisition cost when 
protecting read-mostly objects.

Thanks,

Robert N M Watson
Computer Laboratory
University of Cambridge
Comment 5 Eygene Ryabinkin 2009-04-09 23:56:31 UTC
Robert,

Thu, Apr 09, 2009 at 11:26:04PM +0100, Robert Watson wrote:
> I am working on this, slowly, but it's on the back-burner due to other
> stuff in the queue.  I needed to significantly expand address list
> locking in the work-in-progress, so my current target is to bring the
> changes in before 8.0, then look at an MFC perhaps in time for 7.3.
> This work really relies on having rmlocks in 7.x, which provide a much
> lower acquisition cost when protecting read-mostly objects.

Still: can I somehow help or I should just sit back and relax? ;))
-- 
Eygene
 _                ___       _.--.   #
 \`.|\..----...-'`   `-._.-'_.-'`   #  Remember that it is hard
 /  ' `         ,       __.--'      #  to read the on-line manual
 )/' _/     \   `-_,   /            #  while single-stepping the kernel.
 `-'" `"\_  ,_.-;_.-\_ ',  fsc/as   #
     _.-'_./   {_.'   ; /           #    -- FreeBSD Developers handbook
    {_.-``-'         {_/            #
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:50:11 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 7 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:38:43 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>