All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ben Pfaff <blp@nicira.com>, Jamal Hadi Salim <hadi@cyberus.ca>,
	netdev@vger.kernel.org
Subject: Re: tc: RTM_GETQDISC causes kernel OOPS
Date: Sat, 22 May 2010 11:51:37 +0200	[thread overview]
Message-ID: <4BF7A929.2090007@trash.net> (raw)
In-Reply-To: <1274512687.5020.21.camel@edumazet-laptop>

Eric Dumazet wrote:
> Le vendredi 21 mai 2010 à 15:42 -0700, Ben Pfaff a écrit :
>> Hi.  While working on some library code for working with qdiscs and
>> classes I came upon a kernel OOPS.  Originally I came across it with a
>> 2.6.26 kernel, but I can also reproduce it with unmodified v2.6.34 from
>> kernel.org.
>>
>> At the end of this mail I'm appending both an example of the OOPS and a
>> simple test program that reliably reproduces the problem for me when I
>> invoke it with "lo" as argument.  The program does not need to be run as
>> root.
>>
>> After the OOPS, a lot of networking and other system functions stop
>> working, so it seems to me a serious issue.
>>
>> The null pointer dereference that causes the OOPS is the dereference of
>> the return value of qdisc_dev() in tc_fill_qdisc() in
>> net/sched/sch_api.c line 1163:
>>
>>     1161	tcm->tcm__pad1 = 0;
>>     1162	tcm->tcm__pad2 = 0;
>>     1163	tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
>>     1164	tcm->tcm_parent = clid;
>>     1165	tcm->tcm_handle = q->handle;
>>
>> I am pretty sure about that, because if I add "WARN_ON(!qdisc_dev(q));"
>> just before line 1163 then that warning triggers.
>>
>> Thanks,
> 
> Indeed, thanks for this very useful report !
> 
> We could add a check for TCQ_F_BUILTIN flag, or just make 
> qdisc_notify() checks consistent for both old and new qdisc
> 
> What other people thinks ?

We already use TCQ_F_BUILTIN in tc_qdisc_dump_ignore(), so I
think it would be more consistent than checking for a handle
to use it here as well.

  reply	other threads:[~2010-05-22  9:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-21 22:42 tc: RTM_GETQDISC causes kernel OOPS Ben Pfaff
2010-05-22  7:18 ` Eric Dumazet
2010-05-22  9:51   ` Patrick McHardy [this message]
2010-05-22 12:31     ` jamal
2010-05-23  6:37       ` [PATCH] net_sched: Fix qdisc_notify() Eric Dumazet
2010-05-24  6:11         ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BF7A929.2090007@trash.net \
    --to=kaber@trash.net \
    --cc=blp@nicira.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hadi@cyberus.ca \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.