From: Stanislav Fomichev <sdf@fomichev•me>
To: Willem de Bruijn <willemdebruijn.kernel@gmail•com>
Cc: Stanislav Fomichev <sdf@google•com>,
Network Development <netdev@vger•kernel.org>,
David Miller <davem@davemloft•net>,
Jason Wang <jasowang@redhat•com>,
Jesper Dangaard Brouer <brouer@redhat•com>,
"Michael S. Tsirkin" <mst@redhat•com>,
Eric Dumazet <edumazet@google•com>,
syzbot <syzkaller@googlegroups•com>
Subject: Re: [PATCH net v2 2/2] tun: always set skb->dev to tun->dev
Date: Mon, 7 Jan 2019 14:37:21 -0800 [thread overview]
Message-ID: <20190107223721.GD14250@mini-arch> (raw)
In-Reply-To: <CAF=yD-JLeqAM=LYQCpEry_NBFXDNphjcixGe0+sv0XUqtNCF7w@mail.gmail.com>
On 01/07, Willem de Bruijn wrote:
> On Mon, Jan 7, 2019 at 4:41 PM Stanislav Fomichev <sdf@google•com> wrote:
> >
> > While debugging previous issue I noticed that commit 90e33d459407 ("tun:
> > enable napi_gro_frags() for TUN/TAP driver") started conditionally
> > (!frags) calling eth_type_trans(skb, tun->dev) for IFF_TAP case. Since
> > eth_type_trans sets skb->dev, some skbs can now have NULL skb->dev.
> > Fix that by always setting skb->dev unconditionally.
> >
> > The syzbot fails with the following trace:
> > WARNING: CPU: 0 PID: 11136 at net/core/flow_dissector.c:764
> > skb_flow_dissect_flow_keys_basic include/linux/skbuff.h:1240 [inline]
> > skb_probe_transport_header include/linux/skbuff.h:2403 [inline]
> > tun_get_user+0x2d4a/0x4250 drivers/net/tun.c:1906
> > tun_chr_write_iter+0xb9/0x160 drivers/net/tun.c:1993
> > call_write_iter include/linux/fs.h:1808 [inline]
> > new_sync_write fs/read_write.c:474 [inline]
> >
> > But I don't think there is an actual issue since we exercise flow
> > dissector via eth_get_headlen which doesn't use skb (and hence BPF flow
> > dissector).
>
> Do you mean skb_probe_transport_header?
I was actually thinking about possible future conversion of
eth_get_headlen to be able to run bpf flow dissector, I missed the fact
that we already call dissector via skb_probe_transport_header :-(
>
> if frags, tun_napi_alloc_frags will return napi->skb, which has
> skb->dev set by napi_reuse_skb.
>
> I don't think this is needed.
Ah, indeed, napi_alloc_skb sets proper skb->dev.
next prev parent reply other threads:[~2019-01-07 22:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-07 21:38 [PATCH net v2 1/2] tun: publish tfile after it's fully initialized Stanislav Fomichev
2019-01-07 21:38 ` [PATCH net v2 2/2] tun: always set skb->dev to tun->dev Stanislav Fomichev
2019-01-07 22:30 ` Willem de Bruijn
2019-01-07 22:37 ` Stanislav Fomichev [this message]
2019-01-10 14:26 ` [PATCH net v2 1/2] tun: publish tfile after it's fully initialized 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=20190107223721.GD14250@mini-arch \
--to=sdf@fomichev$(echo .)me \
--cc=brouer@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=jasowang@redhat$(echo .)com \
--cc=mst@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sdf@google$(echo .)com \
--cc=syzkaller@googlegroups$(echo .)com \
--cc=willemdebruijn.kernel@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox