From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933124AbcFBWuk (ORCPT ); Thu, 2 Jun 2016 18:50:40 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:48307 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752502AbcFBWuh (ORCPT ); Thu, 2 Jun 2016 18:50:37 -0400 Authentication-Results: vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=none action=none header.from=fb.com; 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> CC: , From: Calvin Owens Message-ID: <5750B821.7010600@fb.com> Date: Thu, 2 Jun 2016 15:50:09 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.7.2 MIME-Version: 1.0 In-Reply-To: <4912ec551a8ec01181cc3e7ad1e01d3d36758810.1463170976.git.calvinowens@fb.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [2620:10d:c090:200::5:7133] X-ClientProxiedBy: DB6PR0701CA0021.eurprd07.prod.outlook.com (10.168.7.159) To BN4PR15MB0674.namprd15.prod.outlook.com (10.164.62.148) X-MS-Office365-Filtering-Correlation-Id: a24a39f2-8cda-4130-cc06-08d38b384b56 X-Microsoft-Exchange-Diagnostics: 1;BN4PR15MB0674;2:A37BevJsbkxzxS7XrUYk5W4j3rAgw1X+/xGbO1XxNImVPV83mM7tTmQs4bRiAP2HshfNBsBMdHAp93Fi9DP+qfiRzu2pCecw803yQ5/mzqm7LPcpa6xs6tf/Ugd2wNEPfuI0Z572aLTL7fmlvDn/xCn5XQ7NwTEoPe8Dn7UzYUXlgROUhgmpwdGCBgdVwBok;3:z9y4iG2iNenTwf9V2RUPcLPs0hn2yoIGnmSgHydtANAPOiydHvYFb8G27IS99k2ppcce8AHmBWQLhPFhfhF4K98d32Fl/evmrTZXWGehKUvqDf8J10RBSnN3TMUaLqxX;25:+3VV+J3Yhwey8XZfsNk8YgihdhR+rBxdt0nL3orBNz6XJoJQJATNTY8MZHSR/nYZsnIyw2/A/78AyybB10SSQcQjjW1nOeyiwXfkEsCNdL726ny9LHjX8TMsMT+k4vAWFMjJTbjagVu2vZVJTTtwrUNE6FB+5RoBgmgXCOYaeClVPGQ3Sw/KTGA7cZ9Hc2ApHKrmzMAUex7tJiUncFePMERaLEgu2iZeiijiB2piQ1t3l0VnHOXrHEF7ncVnKSY0VQd4BM4VG5rEZ8GNaOF2b9pj+L/QaMIv94fHRmJY+hevMetsbNBfEMWXb2T3FhmpXuB5od1cdT3eyBezZwv2EPQaYHu8Cbogg9pjT0Uy1y4qFED9ywKSVaBpEbd4M8s42z3CPamMtTbk5piTzM22Q501SrgLmMzPe/4XQHTdRmc=;20:nQj7c4XiPReDRU03nfX3hdVAx4WQ96iuUWjEXod3rXi8Be4t29Vlxp1JE/Zx8pUBA8gWjGbuvEMbLrmlTB9LfAQpI9f/WNa6d4MUZr8VFgvBswuBBp+8Y4RayC2a0HFXB2Zefk/ewCLiu21acQ+DqK2+Ze+SmTnnQq4tTooV7bA= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN4PR15MB0674; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(67672495146484); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046);SRVR:BN4PR15MB0674;BCL:0;PCL:0;RULEID:;SRVR:BN4PR15MB0674; X-Microsoft-Exchange-Diagnostics: 1;BN4PR15MB0674;4:45t7lKSqToFt5SisXNfyP6jlbFL8WELb79YO3Xd05J3caUirts7LSrWtkI3Bjbam3VeAOrnCJ3IvVJ6N9kHxvmOzFynDzIaFrbQdbV9MNPq0zKYPZYKEBvZkmDabTiDNdJz8InHDDg1f39yaorAN6DkLsnQ6Ly5QzqU90WcwlUgnCJpsrcU6HN8tCJrxiw9XxZXB5Z1pTE6LJktJkSA9kPOUqbrPbgQrLyqGC99608pLdjhuFNr6W8GpeVW9MAcs2l5cr4+0KFADPK2slIE8nAXtoM31ERrdlvLVkgtWPZ0NXgk5OLGkQ/+buWgGTv76Fd1UF2V0Ez3BPpW4BRVuQfJZoBOTKsZ3TiQus/uvwYeg0yNcFr7ELavsjTHKpWgtgtKUdIELBEcETPRp/0bZZpsUwQhwnKvZZay1L95ojQo= X-Forefront-PRVS: 0961DF5286 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(377454003)(24454002)(80316001)(6116002)(19580405001)(5004730100002)(50466002)(5008740100001)(54356999)(65816999)(87266999)(23746002)(76176999)(19580395003)(2950100001)(59896002)(50986999)(77096005)(33656002)(92566002)(586003)(83506001)(4001350100001)(42186005)(2906002)(8676002)(5001770100001)(86362001)(47776003)(230700001)(81166006)(36756003)(189998001)(4326007)(3826002);DIR:OUT;SFP:1102;SCL:1;SRVR:BN4PR15MB0674;H:[IPv6:2620:10d:c082:10e2:c23f:d5ff:fe6b:54f7];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;BN4PR15MB0674;23:WZRvMr+K70bPnYQGj8+8GkC6IcxCY2fK/mC+5?= =?Windows-1252?Q?Gf3klTvhaUO9mJijYYjZJsFfRonlDhCwHAoOiAAIDYykSrphTgILGJXz?= =?Windows-1252?Q?ZsUiRbLVX8tjNvnxTgph9W9BWK6G42RVpmF9fGXjliWl8dx6P6KOc30u?= =?Windows-1252?Q?jk4lXcT8ZxUHrS0Cs1V8lx4FDDSGyDf8rDwI6CvmBHviVBOhAZh70Hw7?= =?Windows-1252?Q?3bML8nJGpSyBaYZY0AOfouJbR0DLz0m34GWYsBLnxlpC4Fk15AYmpy1k?= =?Windows-1252?Q?G9o2uvMWEv5x09PsUa58zvDUR/mp/sLWARXYgtk6WqJ1MP3Da7oGrYad?= =?Windows-1252?Q?sTgk+cH9Lz9LE+U5xx2og3qw9swTZBY2HKSRu+u4NDb9OQfenq2ulI0n?= =?Windows-1252?Q?NL9qhxHagY4EVLB8qe5EZUplgAMzevjRfew96yvL6msR5WVQWfNAdNaQ?= =?Windows-1252?Q?GruCQtYsMXs1QOTRwnH/U1Kl5by8GLqW4n/iBGk9cD4tWozL3+nNUdZH?= =?Windows-1252?Q?OoYxCETSwTDyZB2STcOjLjCPLSVDWxkdBqBS0eD2Vz02MSaJ3dWyBZgd?= =?Windows-1252?Q?EOAq7zFIcj7FxsC7Pd2E8WDfLJiWI9vlCusxFxZbl1mOna25bGmN6fB6?= =?Windows-1252?Q?sxPBQP+SFMpyVlS5CwhM04L+t8nCueA4d3bM+LV6ytnR77btGVxPUGlN?= =?Windows-1252?Q?/aGttAdy+6dFKCjztMmf8/FvsWQ85SEv2cUlZqj3PFdo+i5ISyje7qRW?= =?Windows-1252?Q?xbtQLHiDiklu2kW75KSvoH6JKxOEtDPWneUnWdz4AxCvzfy0D37qcAUf?= =?Windows-1252?Q?19swzjdWTto1FUYXC+GJV6CYW0HLVCZ2LkIhcMsd3TRUC/RmQ8ww1yfQ?= =?Windows-1252?Q?PEg0uCQcJ3ZdvnKQ1RUamEHDff4aLYyJ13HFxjBPDQTowcSAUCJpbF0j?= =?Windows-1252?Q?d+DVmcmyL2bVQ5yJTwx6I4q4XLAM+YuirvQ9ve/CecwNUxMZE11fKLnk?= =?Windows-1252?Q?0Lt3WS7h13jpWrGKnwZVAOOxjTjSzaMWLO7tsYruL4gGmjrK6AcKFgD/?= =?Windows-1252?Q?NdQvdycFzwa1RyPrFiHyDrWJf4y4y0+mokY?= X-Microsoft-Exchange-Diagnostics: 1;BN4PR15MB0674;5:zRNcskhyrkaGkmtp1sqj3aZI4RpUA+ehpWdQcbt2CtjY79Rvkdbm/I2Q7WOmtrzAiMkQ8EnlRbhXvWotmVYELGDJZPzibPnKCL96f/gt/FReKhuNHGSXGzsWN4uSoIB76AGbRk27Oh0ipT1FxjP2rA==;24:/fRSlq1Ijo1spXWy517AIjwrCJ930WVvKolvqhiktrTiScL360wYsVZX5QDzKuctDI2iyNL+AQ3IZB/hfZqnz4IKHPpULIq+RTopp46Ld+w=;7:C5BNoWhZj2KG2HTVwUU5z0N2D7vamOYatOhdtiRHW645+JBqOtN6nOQmc+3vYm5KYXvpyyr8aTGRuEkJ+nd9crqeVydJQPECzEV6p6GqdDvm8h1mnuFi46rKDZh543B/fsdKzJ4k5wrfIHgbKS6fTa8D0csTjwr9vyzA5xslmta+WzuMq9d8is5Ije0FbBEUn9MI+1nEdibCdI39xxGIdg==;20:0eMkECtdtEO7p/6AncOZsVutG3XFpMS29mg9M4Lbf/ynLP9xXq8W/Jo6Cgetqv1yax5xzxTpFHOH3AuxwYV7JQY2Cg9Y7y51m+Ayw+xgS3L1wSPtpPCobEqMp8Tg/1R3BNaARou+kBBKjnvupndHU24ytjphq88npsRWVUBJDWI= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Jun 2016 22:50:27.2780 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN4PR15MB0674 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-06-02_09:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > 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, >