From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1162828AbcG1BEf (ORCPT ); Wed, 27 Jul 2016 21:04:35 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:45704 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1758277AbcG1BEZ (ORCPT ); Wed, 27 Jul 2016 21:04:25 -0400 Subject: Re: [PATCH] ses: Fix racy cleanup of /sys in remove_dev() To: "James E.J. Bottomley" , "Martin K. Petersen" References: <4912ec551a8ec01181cc3e7ad1e01d3d36758810.1463170976.git.calvinowens@fb.com> <5750B821.7010600@fb.com> <20160615202416.GA52103@calvinowens-mba.dhcp.thefacebook.com> CC: , From: Calvin Owens Message-ID: <617c5fc1-ecc5-c825-ecb9-eeae2b637dfd@fb.com> Date: Wed, 27 Jul 2016 18:04:12 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160615202416.GA52103@calvinowens-mba.dhcp.thefacebook.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [2620:10d:c090:200::e:a5dc] X-ClientProxiedBy: BY2PR11CA0082.namprd11.prod.outlook.com (10.163.150.178) To BLUPR15MB0403.namprd15.prod.outlook.com (10.163.214.149) X-MS-Office365-Filtering-Correlation-Id: a320744c-f1f9-4ce5-99b0-08d3b68319bb X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0403;2:BNHgetyIvaJq+nhEneJPYQINLTCS+Zrfml1VtpvHrgPeB3FOpyLHzfDhUzzp+85RaNq7nMMjSODRrvJ2/OShpEXLwHDQsjZgAD48+1zjy9fKawAvDNPdecpqLjo22eymuyAPfrY4I6vWNNArYON6ZYNFmCaOr8h1aGr/NBKxk/falAlXmgMn02UyVdAOFV2v;3:OSvNbZ2v7MvIOo/zDvNIfbxmsySpCbUiYj8ajOvVPd9M0M34nqRhAAE5VAAWdl+YvJkwKzH7eYiwBhK8Lt2NULVB5L+tBT2psMPRloxanMy3M5Mcp9kAIwUatEmDiXo0 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR15MB0403; X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0403;25:qbuWHySi+BQSe8eXHjNSdKe3Zc04ehEE3WTNcGLZxZujtaDGu6EpgmwUu/cbMayfZmorGnbHWcSEpn3mDxB+tlM7O03isqANuQ/S5jm4zu0N0l6SVY5HcnAcEOS6DoFw5XkOCDdlk1pTxR5vLO3zRLgz7t70ahKaKRCWwbO6Nds21kwhy0MeOZGojeboN2l7D7edJcDxnOkt54JFRVzqnp76wr/pCt3gru15/LriARJtbqR56deISGvGspF0udPZEnzzyt33STmUxW9gOmSJBHN38eEqiw1Hs1/LuNiKf3YqdExC0Z26Af6RMXHGKajUn0CtzDEWtmMUzUxVBZERZbeviBnHahE5eY/CwgFcK1wp1dyaCgqaWdB3rBiLV3jYKQCy+ro0qg5S/ii40yFBe2WPaQ+7angClnhMuK4LiLejgpi24VKhaRX0RIvOJxlnc/2ybdlG4EmPqZt/Kq7Mtui+RkSEINx7GSMuOPD0JeU45QcJQ/DUH5UfqVszkCeXxGOgwEs/2UT/kMQky+r2uE9V/jrGXGKS8IUaUF9j2xBuGbQZWbjj5ovW6SpYG9KrP5TXDQtbcb8rFz1/xDTcPahK4YO2msq1r0PSwUIZ+DFvYpjz24ZEzUcb1VSbIhm5JQa5JwhJPUIrD4iCfqoQcFnE+TZx+1JNzl1lqj87LwfyVvSUYXK8bPsibauoV3/26QJBWPGLbek5/AS+Bchggw==;31:vtwpe/g9ScHmzcmZ7H7UJrjeqnHXSw26HaJ0nTwjaPUaYNkCnzsNDj/441HE1uhqtUC7P9eTZfbVvk3GDZ6QmvXIhinQs/4QbQamZkyz2kGfOufwWhFsm87IyN9vN0Hcub2mOXNg4nKNuwB2e5+RDPueOQv2u0r3N+4rYjVKnxViGG39KGquddPLYI7VToyoMKGctdf9aDSbhjtSUrMjWQ== X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0403;20:4IbBRiMSCB+G4JfcvqBAa0oRXMzbQY981P4sBv0pm11tKbL9P5h/Df6mVHIpmpuaSWaJHh0E02BolcnWq7N2h7d0uIzYywUm+ioS+ZSon2bz9427TIPFTdYKc5sHEPD0ccnl82OnaDezKHgcgyz6Im6Kgc5zRo9WOc8opjhrfmU=;4:wj2UfPcPyQ/ElyCAm9gNcOZzNg+z+NRTCZ3PSlGHA/9jcse345mRgFQR6rke6mXQe/QKqHP3TgJDzo+PvlHo5rNAJUcy5pD2gMD6jraKGWAw33XS+dqRZRPLhJFUWZBTPgqKCvss6rzqe8ur1jGt6k5qvg1iYb9HmBvdiytVVlPxRUcCeWYDixwOkIcyN2iGdUaX6A+6k7XLTWRpdlptO2UIlzyTMMrEbUl3+sdRrdLvleHqHzYD8YNfCk11RJULKJ0eozHHlC71SWRzg8orrUuY6gCPp07WLPYqVK57hjWRr/Yz640tUtl/8VwOBZ4k1ShjFkWV9sz4+dVG2swN7YpBGaSNsGqDCIvv45Q3QEI6zpg6oMSQKg1jsxPtLyzY9LhX6n9iBwrhR+Ele48ItpD0/iIZxoVgcB+MsOBS0GLJ8j7uiG9tCye7wNEVJFPW X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(72170088055959)(67672495146484); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001);SRVR:BLUPR15MB0403;BCL:0;PCL:0;RULEID:;SRVR:BLUPR15MB0403; X-Forefront-PRVS: 00179089FD X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(7916002)(24454002)(199003)(189002)(377454003)(31696002)(33646002)(81156014)(81166006)(19580405001)(50466002)(19580395003)(7736002)(68736007)(76176999)(50986999)(189998001)(42186005)(8676002)(86362001)(54356999)(2950100001)(36756003)(77096005)(31686004)(5001770100001)(97736004)(4001350100001)(230700001)(23676002)(47776003)(1706002)(64126003)(106356001)(83506001)(6116002)(2906002)(305945005)(105586002)(92566002)(101416001)(7846002)(586003)(4326007)(3826002);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR15MB0403;H:[IPv6:2620:10d:c082:10e2:c23f:d5ff:fe6b:54f7];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTFVQUjE1TUIwNDAzOzIzOnVpZitnU1RDOXdiV1VUOGluL0FUbjJNamYw?= =?utf-8?B?bHpXNkZEdGtoZ0tYeUdNMG94dG1rUlRFUFA3ZUF5dWRudGNmSEVGNlliN2li?= =?utf-8?B?RURYeWFCOHBXb1BtQmdSWVdERlZBdWtCdU84aDJhRnJ5dVIvSnl6MG1zOHBl?= =?utf-8?B?aWQzSDlWREpGUlI3dG9HTEZPQ2ZZeUR3Nk0yNzVRS0tySncwNkJjbFU3T3cw?= =?utf-8?B?enllVU1zNCtQYzIyQjJLWDFoYmRTd08rYnl0TyswdDl2OUkxSG1ZSTJ6bnFB?= =?utf-8?B?TFRxTWc2eUNpaEl2WFcyc0s0RGZId2ZmT2o0R1JGN0Ixc011NU9VL3pDRHd6?= =?utf-8?B?bFZEYXlZL1I0SmppTnR1OGRuQTNHazBOT09yc0JXbkwxVHQ4TzdFWGdRZVR4?= =?utf-8?B?TDVnR2hHVWNiOHAzcU9tUTZhT0lPamdxSS9pVVV1ZFUrdzM5S0hWMnNlclEx?= =?utf-8?B?MTRSV1V0dFFCamxOdFV1bW45RTFCRHk1WENJN2hxM1JLUFRWM0hMdFZkSmVT?= =?utf-8?B?ZU92ckp0eTkxd2lnWEI4cGNjMUd4SmRjTFFJalV3Q2hzSzRId0did2xuWk9z?= =?utf-8?B?SjBDU1M1TVVQWFdHMVdaSnpTUWtCcXErTllpek02MDg4dVdLbm1jUDY4dTZt?= =?utf-8?B?czdmUzlPRW5iR01OS05wK0ZQci9OUE9MNFRrWEJMenJWQ0ZKWE9adDE2cWlL?= =?utf-8?B?MHgwZHBQbnhncEJvdTRzcU45UENXVXkzeGZ6WlNHemVwUi9JQTN5SkR2TFF5?= =?utf-8?B?V25tbmV2VkV0bjR5RHFKTUxxY0JmUVU0c1VkdERxRDFoQkhzQ0FwUk9iQnpM?= =?utf-8?B?eFRNa0NtTkZCd0NZSzZWb0NSNVM1cnRlMnh5SHpsaHFnd1cxeG85SVNmMjR4?= =?utf-8?B?dnE2Q3gwd2pqdTVHdy9FamVxenVjRm9KZTc5MU1DS0cvM1lWcC8vUVRUYlVB?= =?utf-8?B?NENSUVJUZ0tkTkI4Q21ZSFVOTkd5M0pXREZETVJZMzd2b1FDWHR4RTVmOGI2?= =?utf-8?B?TjNhZkh6YTNCeE8vVkVNcWw1MGs1TXFNbG1ucEllV1JpM3lybDBWVWt0aWQx?= =?utf-8?B?cGI4M1RiOFR5RFVMbWVQT1NuWlZuSTJHTlhqTlF5elBWeGVPSmpzMlNmUTk3?= =?utf-8?B?Unl1Unp1ejExbWhXOEFyVUtIQXNlaWV1VUdUWkVWK0Jqd0RHMVRNb0YvSXZa?= =?utf-8?B?S25wd2xrZmFGYUtJUjN2ZmIrWmVqTjE2d0J5YkpzY2Z6NElLNFlBQjhrYmQr?= =?utf-8?B?Vmx3dzliZlJ4UEQxWmRpVjdjeWJvWEVLQURkNEF6Ujlwb2R2QXFVOXVxelpa?= =?utf-8?B?bGtTTTFsdndKMWNIejJWNTJxOHY2ZnY5cnBFcGd1MkFsMUVKUDRMbElpbWt5?= =?utf-8?B?OVV2anhzM0ZKU3NIcUJVaWlFQVdHWllHbVJ3OUoyNUtjb1oyK3pnbmhuMk41?= =?utf-8?B?SlRFMVI3Njh3OXU4TUpTNk9zZExNcEpOUkIwRENJKzZjeGp3Q004MEJac1hH?= =?utf-8?B?ejVmTFQ2ZUU3OVRqL0oxSVJSWkg5ZjY2djRMTUFJSkJVUnRWQnhpZnVBQ0pi?= =?utf-8?Q?ENBKVCIOnWDml2DErRRP8jKT83UUHyaP1RC0TnheeKHw=3D?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0403;6:TabdWhZr1EmNygckwaaz6WSmxl1Lr90bQZ/ErpCu/LScc4V3zoudY/qzQ4K8gKyd3MHTWpj26BxeCsr+uorVeHJLwX4ayZbTZ6Wm8E4czBvQeaNGLpEF/361vaZ70zZQAuZTbcKT3Cuw/Frm11F7qB4dH0AwnJcebFR6wLVMpdm0Yw/Z7gN9apGFQn2/vyOMhzD21pp7EdNHgP+v3bH51/KvQS2WPMkF52z3uf8dCstjXbb0N7tTe6N1yzGYklqP/q7VMjHTfSx8DlhEnfVLRnul5KaPV57FMjJ//WHIKSs=;5:Zo6wX2LcwexZpqOePAsKZQHEshMMeYsSeo/Qq23NeHecV1KhBnrY0jnP2BvyviiCNYHvWRjwEeIQlLW+T37Cpvp/eWRt5a0LI2rkOLupNIgJxMiQM5P7GbNFvEFrmv1g3XkyEcDP5P4WL2FryTDksg==;24:r6IOphbOKPCzMLuGmVVbasjx16VlR57YFf6aedYUXsLltWMapjA0AkC9R9R3mQHsWyp8gXQK7j6eQONsurxI1OnucyyTkqK+HtuUf0cmPK8=;7:K7BgP0inFMqjUxdIBZJvRtEYDHLOJDQ7dAAh++1zqQ6Mit5d4209Ex3ZKsoWUmbHJrXjx4Zcdp40t+/9Te41Z/nyla32stxrwEXRd6u/qKHdxaLh2i8EopzC3Ev3IXQ0KFfOqP6ePKMVg6/dKEzNAwW2iZVvnKM+xcg7/0Oc3bJm/f50ltd3LK7E5wbchsQgWdoZ4WjtNFTRleWIsBncYL7ELELLyH9dBg17MiKjxPqEDl7W+umxs+Gj4PNu3+3H SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BLUPR15MB0403;20:KU9PM0+UYgKRJMCYWuw45uSiU0iz7FtMnYFxthpc8jE3+CWNUGFWeQiqBY58Y2g6FYZ7qOT5DWbicO8v0ashuDnbrL6bl/7r9le/YoCdYmHKznoDxjVsDAfPahHpPv5R6nbo7+Huh81P/sxMQpD1W+GxBoqGDnPxqmznkVJM0zI= X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jul 2016 01:04:17.2052 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR15MB0403 X-OriginatorOrg: fb.com X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-27_14:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/15/2016 01:24 PM, Calvin Owens wrote: > On Thursday 06/02 at 15:50 -0700, Calvin Owens wrote: >> On 05/13/2016 01:28 PM, Calvin Owens wrote: >>> Currently we free the resources backing the enclosure device before we >>> call device_unregister(). This is racy: during rmmod of low-level SCSI >>> drivers that hook into enclosure, we end up with a small window of time >>> during which writing to /sys can OOPS. Example trace with mpt3sas: >> >> Ping? > > Any thoughts? Squinting at this more it still seems racy, but a narrow race > is surely better than just blatantly freeing everything while the file is > still exposed in /sys? Is there a better way you'd prefer I accomplish this? > > (I have boxes that OOPS all the time from monitoring code reading the /sys > files, with this patch I haven't seen a single one.) > > Thanks, > Calvin Ping? Thoughts, comments? >>> general protection fault: 0000 [#1] SMP KASAN >>> Modules linked in: mpt3sas(-) <...> >>> RIP: [] ses_get_page2_descriptor.isra.6+0x38/0x220 [ses] >>> Call Trace: >>> [] ses_set_fault+0xf4/0x400 [ses] >>> [] set_component_fault+0xa9/0xf0 [enclosure] >>> [] dev_attr_store+0x3c/0x70 >>> [] sysfs_kf_write+0x115/0x180 >>> [] kernfs_fop_write+0x275/0x3a0 >>> [] __vfs_write+0xe0/0x3e0 >>> [] vfs_write+0x13f/0x4a0 >>> [] SyS_write+0x111/0x230 >>> [] entry_SYSCALL_64_fastpath+0x13/0x94 >>> >>> Fortunately the solution is extremely simple: call device_unregister() >>> before we free the resources, and the race no longer exists. The driver >>> core holds a reference over ->remove_dev(), so AFAICT this is safe. >>> >>> Signed-off-by: Calvin Owens >>> --- >>> drivers/scsi/ses.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c >>> index 53ef1cb..0e8601a 100644 >>> --- a/drivers/scsi/ses.c >>> +++ b/drivers/scsi/ses.c >>> @@ -778,6 +778,8 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev) >>> if (!edev) >>> return; >>> >>> + enclosure_unregister(edev); >>> + >>> ses_dev = edev->scratch; >>> edev->scratch = NULL; >>> >>> @@ -789,7 +791,6 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev) >>> kfree(edev->component[0].scratch); >>> >>> put_device(&edev->edev); >>> - enclosure_unregister(edev); >>> } >>> >>> static void ses_intf_remove(struct device *cdev, >>> >>