From owner-freebsd-net@freebsd.org Mon Dec 3 12:58:38 2018 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3B4CA1327801 for ; Mon, 3 Dec 2018 12:58:38 +0000 (UTC) (envelope-from agapon@gmail.com) Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 1D58081403; Mon, 3 Dec 2018 12:58:37 +0000 (UTC) (envelope-from agapon@gmail.com) Received: by mail-lf1-f47.google.com with SMTP id f23so9017723lfc.13; Mon, 03 Dec 2018 04:58:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=0lkxHgPJEuEnaRkVCIYnNtLwcMPl01yZP2v12Q3g/4g=; b=FbMvVZqZGWZNeye3KDl58qkNsBPMySc3+a+mi9WtC6CVRsOdstRDNHOWqSUhCKfjSZ 5xORHU2W1yM7l/Uu4Ew63LOrcYeKlHFh/E0D1mATYXQtq8YLoO3rp5FeCueuaNLLvIo1 wUIL2s0uV+KloWJ+y477LqYHfvj3iSs8SSMHVpvMXimJqWPu4D+XLHfKJL1wXE7Aw028 mRfrKAa7dZZpItOUkVL8b/YXlAbp3PONOqIWsELow35lMwIeVIh934MpoISMnt2BvlCO l53duANy5aCBpQAA8z6v7DoW7pH2dnApERwPZgvB7ItP/wJTl43ZBysUEwnUaZhS/pQ9 p56g== X-Gm-Message-State: AA+aEWZ8ha/8bz7Qp8hYSxN5uacr7Kv+VHEM1Cxztb3Rb6LUjPbixfcf LzHzj4edSIlQPrtRDPHb8q6sQmF0 X-Google-Smtp-Source: AFSGD/V1ZoL8NkEI2NQDHmsh6jO/CerwnzYssPKJ1dbPNiVt0aTxiZXPgSlqIpv+LyHBf+EASW/lZQ== X-Received: by 2002:a19:d90c:: with SMTP id q12mr8672735lfg.24.1543840474499; Mon, 03 Dec 2018 04:34:34 -0800 (PST) Received: from [192.168.0.88] (east.meadow.volia.net. [93.72.151.96]) by smtp.googlemail.com with ESMTPSA id r4sm2333458lfe.60.2018.12.03.04.34.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Dec 2018 04:34:33 -0800 (PST) Subject: Re: lagg: a race between ioctl and clone_destroy From: Andriy Gapon To: freebsd-net@FreeBSD.org References: <8fcf453e-c326-eb14-367a-655cbee5d088@FreeBSD.org> Openpgp: preference=signencrypt Autocrypt: addr=avg@FreeBSD.org; prefer-encrypt=mutual; keydata= xsFNBFm4LIgBEADNB/3lT7f15UKeQ52xCFQx/GqHkSxEdVyLFZTmY3KyNPQGBtyvVyBfprJ7 mAeXZWfhat6cKNRAGZcL5EmewdQuUfQfBdYmKjbw3a9GFDsDNuhDA2QwFt8BmkiVMRYyvI7l N0eVzszWCUgdc3qqM6qqcgBaqsVmJluwpvwp4ZBXmch5BgDDDb1MPO8AZ2QZfIQmplkj8Y6Z AiNMknkmgaekIINSJX8IzRzKD5WwMsin70psE8dpL/iBsA2cpJGzWMObVTtCxeDKlBCNqM1i gTXta1ukdUT7JgLEFZk9ceYQQMJJtUwzWu1UHfZn0Fs29HTqawfWPSZVbulbrnu5q55R4PlQ /xURkWQUTyDpqUvb4JK371zhepXiXDwrrpnyyZABm3SFLkk2bHlheeKU6Yql4pcmSVym1AS4 dV8y0oHAfdlSCF6tpOPf2+K9nW1CFA8b/tw4oJBTtfZ1kxXOMdyZU5fiG7xb1qDgpQKgHUX8 7Rd2T1UVLVeuhYlXNw2F+a2ucY+cMoqz3LtpksUiBppJhw099gEXehcN2JbUZ2TueJdt1FdS ztnZmsHUXLxrRBtGwqnFL7GSd6snpGIKuuL305iaOGODbb9c7ne1JqBbkw1wh8ci6vvwGlzx rexzimRaBzJxlkjNfMx8WpCvYebGMydNoeEtkWldtjTNVsUAtQARAQABzR5BbmRyaXkgR2Fw b24gPGF2Z0BGcmVlQlNELm9yZz7CwZQEEwEIAD4WIQS+LEO7ngQnXA4Bjr538m7TUc1yjwUC WbgsiAIbIwUJBaOagAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRB38m7TUc1yj+JAEACV l9AK/nOWAt/9cufV2fRj0hdOqB1aCshtSrwHk/exXsDa4/FkmegxXQGY+3GWX3deIyesbVRL rYdtdK0dqJyT1SBqXK1h3/at9rxr9GQA6KWOxTjUFURsU7ok/6SIlm8uLRPNKO+yq0GDjgaO LzN+xykuBA0FlhQAXJnpZLcVfPJdWv7sSHGedL5ln8P8rxR+XnmsA5TUaaPcbhTB+mG+iKFj GghASDSfGqLWFPBlX/fpXikBDZ1gvOr8nyMY9nXhgfXpq3B6QCRYKPy58ChrZ5weeJZ29b7/ QdEO8NFNWHjSD9meiLdWQaqo9Y7uUxN3wySc/YUZxtS0bhAd8zJdNPsJYG8sXgKjeBQMVGuT eCAJFEYJqbwWvIXMfVWop4+O4xB+z2YE3jAbG/9tB/GSnQdVSj3G8MS80iLS58frnt+RSEw/ psahrfh0dh6SFHttE049xYiC+cM8J27Aaf0i9RflyITq57NuJm+AHJoU9SQUkIF0nc6lfA+o JRiyRlHZHKoRQkIg4aiKaZSWjQYRl5Txl0IZUP1dSWMX4s3XTMurC/pnja45dge/4ESOtJ9R 8XuIWg45Oq6MeIWdjKddGhRj3OohsltKgkEU3eLKYtB6qRTQypHHUawCXz88uYt5e3w4V16H lCpSTZV/EVHnNe45FVBlvK7k7HFfDDkryM7BTQRZuCyIARAAlq0slcsVboY/+IUJdcbEiJRW be9HKVz4SUchq0z9MZPX/0dcnvz/gkyYA+OuM78dNS7Mbby5dTvOqfpLJfCuhaNYOhlE0wY+ 1T6Tf1f4c/uA3U/YiadukQ3+6TJuYGAdRZD5EqYFIkreARTVWg87N9g0fT9BEqLw9lJtEGDY EWUE7L++B8o4uu3LQFEYxcrb4K/WKmgtmFcm77s0IKDrfcX4doV92QTIpLiRxcOmCC/OCYuO jB1oaaqXQzZrCutXRK0L5XN1Y1PYjIrEzHMIXmCDlLYnpFkK+itlXwlE2ZQxkfMruCWdQXye syl2fynAe8hvp7Mms9qU2r2K9EcJiR5N1t1C2/kTKNUhcRv7Yd/vwusK7BqJbhlng5ZgRx0m WxdntU/JLEntz3QBsBsWM9Y9wf2V4tLv6/DuDBta781RsCB/UrU2zNuOEkSixlUiHxw1dccI 6CVlaWkkJBxmHX22GdDFrcjvwMNIbbyfQLuBq6IOh8nvu9vuItup7qemDG3Ms6TVwA7BD3j+ 3fGprtyW8Fd/RR2bW2+LWkMrqHffAr6Y6V3h5kd2G9Q8ZWpEJk+LG6Mk3fhZhmCnHhDu6CwN MeUvxXDVO+fqc3JjFm5OxhmfVeJKrbCEUJyM8ESWLoNHLqjywdZga4Q7P12g8DUQ1mRxYg/L HgZY3zfKOqcAEQEAAcLBfAQYAQgAJhYhBL4sQ7ueBCdcDgGOvnfybtNRzXKPBQJZuCyIAhsM BQkFo5qAAAoJEHfybtNRzXKPBVwQAKfFy9P7N3OsLDMB56A4Kf+ZT+d5cIx0Yiaf4n6w7m3i ImHHHk9FIetI4Xe54a2IXh4Bq5UkAGY0667eIs+Z1Ea6I2i27Sdo7DxGwq09Qnm/Y65ADvXs 3aBvokCcm7FsM1wky395m8xUos1681oV5oxgqeRI8/76qy0hD9WR65UW+HQgZRIcIjSel9vR XDaD2HLGPTTGr7u4v00UeTMs6qvPsa2PJagogrKY8RXdFtXvweQFz78NbXhluwix2Tb9ETPk LIpDrtzV73CaE2aqBG/KrboXT2C67BgFtnk7T7Y7iKq4/XvEdDWscz2wws91BOXuMMd4c/c4 OmGW9m3RBLufFrOag1q5yUS9QbFfyqL6dftJP3Zq/xe+mr7sbWbhPVCQFrH3r26mpmy841ym dwQnNcsbIGiBASBSKksOvIDYKa2Wy8htPmWFTEOPRpFXdGQ27awcjjnB42nngyCK5ukZDHi6 w0qK5DNQQCkiweevCIC6wc3p67jl1EMFY5+z+zdTPb3h7LeVnGqW0qBQl99vVFgzLxchKcl0 R/paSFgwqXCZhAKMuUHncJuynDOP7z5LirUeFI8qsBAJi1rXpQoLJTVcW72swZ42IdPiboqx NbTMiNOiE36GqMcTPfKylCbF45JNX4nF9ElM0E+Y8gi4cizJYBRr2FBJgay0b9Cp Message-ID: Date: Mon, 3 Dec 2018 14:34:32 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <8fcf453e-c326-eb14-367a-655cbee5d088@FreeBSD.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 1D58081403 X-Spamd-Result: default: False [-3.95 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17]; TO_DN_NONE(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; MX_GOOD(-0.01)[cached: alt3.gmail-smtp-in.l.google.com]; NEURAL_HAM_SHORT(-0.96)[-0.964,0]; FORGED_SENDER(0.30)[avg@FreeBSD.org,agapon@gmail.com]; RCVD_TLS_LAST(0.00)[]; R_DKIM_NA(0.00)[]; FORGED_RECIPIENTS(0.00)[freebsd-net@FreeBSD.org,jpaetzel@freebsd.org ...]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[avg@FreeBSD.org,agapon@gmail.com]; TO_DOM_EQ_FROM_DOM(0.00)[]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; MID_RHS_MATCH_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[FreeBSD.org]; RCPT_COUNT_ONE(0.00)[1]; IP_SCORE(-0.98)[ipnet: 209.85.128.0/17(-3.51), asn: 15169(-1.29), country: US(-0.09)]; RCVD_IN_DNSWL_NONE(0.00)[47.167.85.209.list.dnswl.org : 127.0.5.0]; RWL_MAILSPIKE_POSSIBLE(0.00)[47.167.85.209.rep.mailspike.net : 127.0.0.17] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Dec 2018 12:58:38 -0000 On 30/11/2018 14:27, Andriy Gapon wrote: > Q1. Is the described race plausible? > > Q2. If yes, then has this class[*] of races been fixed by the epoch mechanism? > I suspect that lagg_ioctl() can still have that race if it's called concurrently > with lagg_clone_destroy(). So, to re-iterate, I think that the code currently allows for the following race with respect to drivers that use if_clone_simple mechanism (at least). Thread T1 calls ifioctl with, for instance, SIOCGIFMEDIA parameter. T1 calls ifunit_ref(), finds the interface with !(if_flags & IFF_DYING). T1 increments the interface's reference count and proceeds to call ifhwioctl() and then the interface's (driver's) if_ioctl method. Enter thread T2. T2 calls ifioctl(SIOCIFDESTROY) on the same interface. T2 invokes if_clone_destroy() that looks up the interface by name and increments its reference count. Then, T2 calls if_clone_destroyif() that calls ifc_simple_destroy(). The latter calls ifcs_destroy method on the interface. >From here on we consider driver-specific code that, obviously, varies from driver to driver. But after having reviewed a handful of drivers that use if_clone_simple I see that all of them have the same pattern. So, T2 calls ifcs_destroy. A driver's ifcs_destroy would handle its internal state. Then, it would typically call if_free() on the interface. Since the interface at this point has multiple outstanding references, including one taken by T2 itself, it is not actually freed. It's just marked as IFF_DYING. Also, its reference count is decremented by one, so that it can be actually freed after T2 and T1 release their references. Afterwards, ifcs_destroy would typically free if_softc. At this point the driver's if_ioctl method is being executed by T1. The method can access if_softc that has been freed by now. So, that's the race. Any internal locking on the driver level does not help, because the lock would be destroyed and freed together with the if_softc in ifcs_destroy. So, if_ioctl attempting to get that lock is the same kind of the problem. Any thoughts and suggestions? My idea is that there should be something like 'if_freed' or 'if_destroyed' method that could be used by drivers for their cleaning up. That method would be called from if_destroy() when we really know that the interface has no references and, thus, no threads are accessing it. Then it must really be safe to destroy the softc as well. How does this sound? Thanks! -- Andriy Gapon