From owner-freebsd-pf@FreeBSD.ORG Tue Apr 17 08:38:32 2012 Return-Path: Delivered-To: freebsd-pf@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 629AB106566B; Tue, 17 Apr 2012 08:38:32 +0000 (UTC) (envelope-from ermal.luci@gmail.com) Received: from mail-iy0-f182.google.com (mail-iy0-f182.google.com [209.85.210.182]) by mx1.freebsd.org (Postfix) with ESMTP id 17F9E8FC22; Tue, 17 Apr 2012 08:38:32 +0000 (UTC) Received: by iahk25 with SMTP id k25so11287758iah.13 for ; Tue, 17 Apr 2012 01:38:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=kgIs0/IBW3an/l9B20oqfqRzaJZ66IxvNgQFbCoEreY=; b=Pps23QXYFah9XVwFvM7EMIKeZ3CRtwr2fTAXPc0KhHsbk0ZV+JJeMJUVQ/Chq6Km4L EY++PKpiByv41MDB3zLbQwbMCiZMfwj8U8Tgr/MO01pVBj2iymqsw6yeZK3e/sGuqUyI c4Qmf0u+VkxrkZjFPLbd6aonU04Et6fTgs0Amwp3CIO3Oval/J5ats63afR8MHiVlwnf jesaNkNOWO4zw4317YFEPPUY/cB+bnL8hKepbGCuIq0GFDyPw9BYbw0B+LHtxAeV6eWP a2bmPrYWWAMOmv9hrrNJ9dkT2jZFTW4TmscpHNf3cGKYc09KPujwHJIH7agcL+NWXeKP 7+6A== MIME-Version: 1.0 Received: by 10.50.41.168 with SMTP id g8mr8282736igl.17.1334651911767; Tue, 17 Apr 2012 01:38:31 -0700 (PDT) Sender: ermal.luci@gmail.com Received: by 10.231.243.65 with HTTP; Tue, 17 Apr 2012 01:38:31 -0700 (PDT) In-Reply-To: <20120417081406.GA93887@glebius.int.ru> References: <201204151200.q3FC0LT5085161@freefall.freebsd.org> <20120416185949.GC92286@FreeBSD.org> <20120417081406.GA93887@glebius.int.ru> Date: Tue, 17 Apr 2012 10:38:31 +0200 X-Google-Sender-Auth: H2tZtDpQ7X1xpWKbcYvEuTvWsJs Message-ID: From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= To: Gleb Smirnoff Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-pf@freebsd.org Subject: Re: kern/164402: [pf] pf crashes with a particular set of rules when first matching packet arrives X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Apr 2012 08:38:32 -0000 2012/4/17 Gleb Smirnoff : > On Tue, Apr 17, 2012 at 10:06:15AM +0200, Ermal Lu?i wrote: > E> 2012/4/16 Gleb Smirnoff : > E> > On Sun, Apr 15, 2012 at 12:00:21PM +0000, Gleb Smirnoff wrote: > E> > T> =A0On Sun, Apr 15, 2012 at 11:10:03AM +0000, Gleb Smirnoff wrote: > E> > T> =A0T> =A0 =A0I have a vague suspicion on what is happening. Your = description of > E> > T> =A0T> =A0the problem looks like if a packet processing in the ker= nel has entered > E> > T> =A0T> =A0an endless loop. > E> > T> =A0T> > E> > T> =A0T> =A0 =A0Looking at pf_route() I see such possibility. From O= penBSD we have > E> > T> =A0T> =A0this protection against endless looping: > E> > T> =A0T> > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0if ((*m)->m_pkthdr.pf.routed++ > 3) { > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m0 =3D *m; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*m =3D NULL; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto bad; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0} > E> > T> =A0T> > E> > T> =A0T> =A0In our code this transforms to: > E> > T> =A0T> > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0if (pd->pf_mtag->routed++ > 3) { > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m0 =3D *m; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*m =3D NULL; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto bad; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0} > E> > T> =A0T> > E> > T> =A0T> =A0The root difference between storing the tag on mbuf and = on pfdesc > E> > T> =A0T> =A0is that we lose pfdesc, and thus the tag, when we enter = pf_test() > E> > T> =A0T> =A0recursively. And pf_route() does this recursion: > E> > T> =A0T> > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0if (oifp !=3D ifp) { > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (pf_test(PF_OUT, ifp,= &m0, NULL) !=3D PF_PASS) { > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto bad= ; > E> > T> =A0T> =A0 =A0 =A0 =A0 =A0.... > E> > T> > E> > T> =A0On second look I see that my suspicion may not be true. In the > E> > T> =A0beginning of pf_test() we do pf_get_mtag() which preserves alr= eady > E> > T> =A0present tag if there is one. > E> > > E> > Further investigation showed that problem exist when route applied > E> > ends in lo0, and packet passes to if_simloop(). There all mtags are > E> > stripped from the mbuf, including the pf mtag. Then packet is again > E> > processed by ip_input() again entering pf(4), if it again matches > E> > a routing rule, then we got an endless loop. > E> > > E> > We can try to fix this applying MTAG_PERSISTENT to the pf(4) tag id. > E> > E> That seems like the best fix for this case. > > In this case crash or freeze is fixed, but still packet is dropped. Examp= le > of such rule: > > pass in on igb0 fastroute proto tcp from any to $localip > > Anyway, dropping packets is much better than crashing. > Actually after some coffee :) i think its better marking the packet with M_SKIP_FIREWALL since it has already taken its decision on this packet. The simloop consumers seem to be just facilities of how things work from what i can see. So just delivering the packet by sending skipping the firewalls seems more sensibile! Though the persistent case for the tags should be revisited since it may fix some other issues with pf(4) tags, and some others. > -- > Totus tuus, Glebius. --=20 Ermal