Received-SPF: pass (google.com: domain of linux-kernel+bounces-19420-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <a84b2797-2008-45d6-9ca3-c72666d3c419@virtuozzo.com>
Date: Mon, 8 Jan 2024 19:26:09 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] neighbour: purge nf_bridged skb from foreign device neigh
Content-Language: en-US
To: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org, kernel@openvz.org
References: <20240108085232.95437-1-ptikhomirov@virtuozzo.com>
 <20240108111504.GA23297@breakpoint.cc>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
In-Reply-To: <20240108111504.GA23297@breakpoint.cc>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?UVNrTXREWnZ1MUE1MWd1bU56VUhFYkwrdU5yMG5Cc09iTDV3RHdLR0dFcDdp?=
 =?utf-8?B?K0pDV1VzMUJwZlYzWG1CWlRiMlp3Tkp2a3ZKWXJ3Ri9BV0FEOHJGcjU2N21o?=
 =?utf-8?B?aWYyVEl3ek5ZbW5MNW5Zdko2YmMwb2RPdU1vaTFIbjlJbmhOZUVxeU9xdm9z?=
 =?utf-8?B?eDlYZzU4STE2OGZCWWZQeFlhaWNCWjdKZXRwNGxZdy9ISmIvM0l0bldzWS9p?=
 =?utf-8?B?Z2piYmlJcXNmYnY0NGRnTFlRb09oQkcrYmZlbjFLODR4akMyeEFWbndTc3p6?=
 =?utf-8?B?c1ZDbnBtcjZVZ1VVN0JrTWNMYmRlazZWcm1RNDVHMEdlYTl3Q3A3MmpoTzA3?=
 =?utf-8?B?N3E4SzlEaUk4UTV3UWFFZTArSTdSSWh0a1RyYTY1K1V6d3IvNVBNL3lNaU9y?=
 =?utf-8?B?WkRwcDUyUktjeXRxZXFDbHlJV1Qxa0RaQ0ExalF1TzlMZy80L0dsK3h2MlFW?=
 =?utf-8?B?a1ZkYXh6akROVXBKd3orVmNDOG9BZ2JwejVZc1JRaXVIUjcxZFVMRzh4QTcv?=
 =?utf-8?B?UlkvV1pRWjRZb1BUaVBwc2tRV05OZ1g3SWcwYUtrRFN2TnQ2U2pWeFUzUGw5?=
 =?utf-8?B?cy9UTEkycnpBT2VWcW5RdjJoSlJYOURsazc3QjNDZ1BWVEVEZmhpY0NpYThv?=
 =?utf-8?B?MUZGSTQzQ0FFTjdWczQwQ2Z0R0U3SURxdzVqY21Sa1IrenREZ0dwUVROSzJH?=
 =?utf-8?B?dWxPL05RdWFCQWladVdSZEg0ZGdOQXhJSEs2b2ZpdWIrNmxZZHA5cEthcmNy?=
 =?utf-8?B?U3MyVHgrc2JvOWlHODJOdzVmNEVOc3ltMUtPUUZXN0xhYzN5d2V4c0l2Ry93?=
 =?utf-8?B?cGdDSythY3k1eU5veExObG01ZVdkSVA2Vm9WbWJ2RmszMTdkNVNWOXBCa0dL?=
 =?utf-8?B?SzlsQVJCNENMRlZVNkhlV05CSjd3ZlNwWmNCelR3cEp4MDZ4TGFlTHNEM1Zt?=
 =?utf-8?B?MnFNUXZFSG15SnhBNzNiTHRsOEtqc3hMUXhoTTU0QWdZV3lKVHg0ZlZKTXZ5?=
 =?utf-8?B?bEtBODdXeHI3ZjhxU2pndWZvWlpvSGIwb2M0VGtNM08rbTFaNnZKQktkRzRt?=
 =?utf-8?B?cFBCa0ZudmV6NGw5cmp6UHpMekVHRC9YMldyZklodTByaDRWOFNkL1A4bEZk?=
 =?utf-8?B?ZmYzbjFkdUlHQnZCeHRIdExhb0dKZnJyY3RsWGhmUWhZOUhsNlFUNEZLbVN2?=
 =?utf-8?B?RTlvUzNBd3d3aVl2Qld2MS9IamZpblNoVHpFMjRtU1VjWjZZZWV2bnJxWkJ4?=
 =?utf-8?B?ajhkaEQ2SkJmTUptcWMrbk96R3NjQk1zTHQzOVJRT2FDc2VJVEV6SUJPUUdD?=
 =?utf-8?B?S0lkaTRWQ1RvNzdrMDZvdnZwNTRTdnJMQ2ticUJ5T2RHTEVCbkFUVWo5UDNw?=
 =?utf-8?B?N01aQUpMSzlLZndxR3hOWWkvVjJMOXBjRHp1LzZHOHlyL0pGT2VLTUlnOUY1?=
 =?utf-8?B?K1Z0WVB0VVBTS1BjODdEVVVLOWtsSFNWMm9tMmttZVFxWDFsRnJnbWUzQVRa?=
 =?utf-8?B?N0tyMjl5WVNlbDhiQnFGeDBKaEx0b1I4d3hrSWlmYnB4alVVVHZLU0VxSWdn?=
 =?utf-8?B?ZW94aitpL1ZxTEdCYWxmK2tIR09tZkF5QTFzWGFEZGRqVm9XaktZOGtWY21P?=
 =?utf-8?B?b2RtWTZJZjJFVjBZa0oxclVZeVhoLzIwZ2w5bWQ3UFVDQ0VKRGUxSms3S0JN?=
 =?utf-8?B?OGRFMHlYSkhsZ0pNb1pEak44NjJUWit3Ty85dlNxY3JybERCb2k3WER3blR6?=
 =?utf-8?B?cVorc2RvTUhlNnNSWVl1YUM2MlhtaVB5V3kyZDJwbTdkMG5XSGZGRXNYVDl2?=
 =?utf-8?B?bmp3RHhhM3ltc0FSRVRpeDlhTnJSd3NJMnZHNXdtcW9TL09RV1NySldpR2NC?=
 =?utf-8?B?dk9EbFZxZ3RESW1UWSswbVNNRE5uYU95SHd2Y3FrNlE1YVlYU0RKc2JVMVI1?=
 =?utf-8?B?Y0FnYlRGT25LL0JYV3hiajEzZXArZDlGVXFwQklRbVk3STJOY2xIcktxVjZj?=
 =?utf-8?B?a24ydDRsdEx1dW1KNC9tQU9EdTJaMkYrbGZOMVRqbHJLVytFMjBvWjVpakpi?=
 =?utf-8?B?bVlBQlduWDdDeTR3K0dCVk9kcmZOUzVyVGxlNnBzZ0tMVWdjV2NkZ0w2dHoz?=
 =?utf-8?B?ZkJqNUJ3clZRWU5IT2g0emxKdmhIR0x5dEl2THhoajRqUjh6MHA5cTEzM1R3?=
 =?utf-8?B?MGVLSXdJY3JHRmlremZZdzNxeTBrdGQvWmJlYmJmSnJ1Q1V3S3RXdWh1aE1u?=
 =?utf-8?B?dWt1dkJwZkFYdGs2V1poa1phQXVBPT0=?=
X-OriginatorOrg: virtuozzo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 285c91a3-2eb1-474f-b794-08dc103ca094
X-MS-Exchange-CrossTenant-AuthSource: DU0PR08MB9003.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jan 2024 11:26:16.0576
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +mBFXLPxga2G93KyvHmjcsiDZ4gmJIrmIBC2ITWhBxJRsYh9jRvsWxCqAulCCC+8syO3k+qTi/ofi2N5taByr/8iLbVmeRhKMkisQ3EdC3E=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS2PR08MB9522


On 08/01/2024 19:15, Florian Westphal wrote:
> Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
>> An skb can be added to a neigh->arp_queue while waiting for an arp
>> reply. Where original skb's skb->dev can be different to neigh's
>> neigh->dev. For instance in case of bridging dnated skb from one veth to
>> another, the skb would be added to a neigh->arp_queue of the bridge.
>>
>> There is no explicit mechanism that prevents the original skb->dev link
>> of such skb from being freed under us. For instance neigh_flush_dev does
>> not cleanup skbs from different device's neigh queue. But that original
>> link can be used and lead to crash on e.g. this stack:
>>
>> arp_process
>>    neigh_update
>>      skb = __skb_dequeue(&neigh->arp_queue)
>>        neigh_resolve_output(..., skb)
>>          ...
>>            br_nf_dev_xmit
>>              br_nf_pre_routing_finish_bridge_slow
>>                skb->dev = nf_bridge->physindev
>>                br_handle_frame_finish
>>
>> So let's improve neigh_flush_dev to also purge skbs when device
>> equal to their skb->nf_bridge->physindev gets destroyed.
> 
> Can we fix this by replacing physindev pointer with plain
> ifindex instead?  There are not too many places that need to
> peek into the original net_device struct, so I don't think
> the additional dev_get_by_index_rcu() would be an issue.

I will work on it, thanks for a good idea!

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.