Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <85b67cd9-2d00-f2f2-1335-efab7240d65d@oracle.com>
Date:   Thu, 10 Nov 2022 15:47:10 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH] jfs: makes diUnmount/diMount in jfs_mount_rw atomic
Content-Language: en-US
To:     Oleg Kanatov <okanatov@gmail.com>
Cc:     Oleg Kanatov <okanatov@astralinux.ru>,
        jfs-discussion@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        lvc-project@linuxtesting.org
References: <20221028122254.21081-1-okanatov@astralinux.ru>
From:   Dave Kleikamp <dave.kleikamp@oracle.com>
In-Reply-To: <20221028122254.21081-1-okanatov@astralinux.ru>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RkpzUTRYbWNpL3ZEVHFmTmNvV2U5M0lVTVhldGh1ZFVDM2VMOEp1eC85Q1Fl?=
 =?utf-8?B?WXBJM09QZ2p5SXJ6NW5pWWN3a1hpQkZ5ZkUvelpwcGplRFNhYlNCL2F2RjJD?=
 =?utf-8?B?YUpIcXFrb2o4bEp2UXJ4OHdaakppV29uVlR5YmZ3b2U0MzVJM2hnSVNLTkJY?=
 =?utf-8?B?ZXdlb2NySGtOek9hYVBzRHVTZytOWm9lM28reXZ3YmlqVVN3S1E4elRucTJk?=
 =?utf-8?B?YUZWMlQvUkRzaVlNQlI1VDAvMy82UEZYM0FlUHlWaC9EYURYbUg0UGFLT0pp?=
 =?utf-8?B?WE1DNnZZbkE5Y2ppWHhNaEhPSmtpRmo5LzY0Q2F1ck5qK2JDc3V1ZmNhNHFQ?=
 =?utf-8?B?V2NLSEg5SEdtT0hBOW1naGNNS09ZeWNrQUhpQVo5ampnbFdKV3pJUG1oT3VZ?=
 =?utf-8?B?aEVoaW1DdXNQdE05Tm54TzBIM1RzRzhLMGlKeE9pcDlNd0o1MTVQVFhQVFRi?=
 =?utf-8?B?QnhaMHR4LzJ2RlFqeFAvc2FCR3hOTGVUV0xxV1ZTMlF2M3hFaVJZcm1kVU1u?=
 =?utf-8?B?VmtUd29xazNwcUxORTB5WFl6WEZjYy8rZ0czNEVYUGt0dzVlNUlzRFI5cUYx?=
 =?utf-8?B?ODczd3N5TUYvaCt1QlRScENSUXBkTTBUUWdlMEVNVml3eHVwQ1MwNXphWVR2?=
 =?utf-8?B?akViV3JkdmZjbkxpTmNWOFRqZmJKU2lFcE1uWTlyQjRHbVJpZm9FUGVkOGRa?=
 =?utf-8?B?WVAyQVBPVkl2bTVONmtaZ1FhMjk3MWpBam84N2xTcWJiWFVDbWh6Tm9Rendr?=
 =?utf-8?B?UnVwaFZiekszdkxhQ3V4Z0tQUmJyTnp3MzBRVUl1TjVwWk5ldjcxOEs1NXpH?=
 =?utf-8?B?c2FvK2kvUURxVzNONmFrV3NuS1RYN3NyNXdCVmx6RVplbTJUS2FuVDArZHN6?=
 =?utf-8?B?TnZldWp0bGJPLzlWSTFEVGN5T3N0elZXUkFHNnVvTjVyN0RKREdDb05RZzRG?=
 =?utf-8?B?cG1pVkZQVHlLLzVoK25USVhmdGRubEJvZEZvNmwvNHVURWdTUGV4dEpjZ1hs?=
 =?utf-8?B?Z0huVU4rSkxMV05GOEswZllXN0dlbHBHdjcrbGhxOEhVaTQzbmREVkZJc0RU?=
 =?utf-8?B?MjZNTmh6Y3NHWmIwdk1SSTl3WE12VE0wdTF4c1E3dENXb1dwKzFmYXpaRlpo?=
 =?utf-8?B?MVBLbGZxMGUwd2l3aWVNWEtwdm4yNXBqd2dPV042dncyRjZ6Vkl2Y1hyQTJl?=
 =?utf-8?B?TlJVK3UzRnplT3dpWEM1N3FjVnYxMEs2RlAyK24wcmxwakorcnFFSzhJbVJ2?=
 =?utf-8?B?MGFneTRLNDljWEtSdGRIdGtYQXRCNHhlMU5PQ3dvQkQ4bGNvSmR5RDVuWlNH?=
 =?utf-8?B?ek1kRjM1c05WZ3NacFhxRTNDRm85UWFKK3RvaUFZYmYzL2R2emhyWmU5ZlpM?=
 =?utf-8?B?bXF3NDZEV1RBN3VmRXNsdmV3MkNIdUdta2Z0L0JFUjg0bS80K2k0R3htMEI5?=
 =?utf-8?B?U0tmUlJPVzNla2xiNkE5dlBQdzVLQ0d3QmdMdXJtbGhUeUlKMk1OWGFCSHVl?=
 =?utf-8?B?QXFTUWpmY0d3czViMk9PUkNSVTd0RnpDOVRHTUR3aHpYNU02RlROYkh6QUtB?=
 =?utf-8?B?bXBMRkY1SW1iQWRlWHFDb20xLzJmd3JGd0RDZ0hwT3d5V2w0TUdHckFwTkU3?=
 =?utf-8?B?WTVLN3YzZ3NYUm1rSmQvakYrckhQTWJVL1MxNndybXBUak9SNTZEcVIzdTRv?=
 =?utf-8?B?dHdiTU9BY0QxWldrQ29lUFVoeDg2eFRZeHpNMGtRcVZBY1FjdGRIMkJYb0FN?=
 =?utf-8?B?ZDJWajBxRVBtOWhDbGRSdFp2cWtrcGFOVzZXMHlWNXlORmhwaEZiZFQvSlN1?=
 =?utf-8?B?bDJ5bVViSUlldWNQbkRGQlZGTzR6NkhuWlhGSlIxZG5Bc1BKMVNXaVdZdlVR?=
 =?utf-8?B?RXNmUzNxWHczV1k1MmU2NEVuTWEyMXFiRkpRY2FSa3VpTTc2dUxTVXlSbndN?=
 =?utf-8?B?VDZmMXJiUjkrTWRGYU5VSVlnakw3amVhN2lVdExOTGFNOVRIYzQ1ZE1hemVq?=
 =?utf-8?B?b25YODF2ZUlEY2hlN01oTXRicGZxUDd0RWVZaGR6em9XVnA4RnpJTnNDSlZV?=
 =?utf-8?B?V29NUzBjN0poSFlpR1dEUXEvVGdIWTJBK1V3RFpOdHlCUTFYbldBSXg1eVkv?=
 =?utf-8?B?L0pJbDF6TGFJL0V6MjZLR1RCcHltcHFaL0x2V21QZmxlS2J6ZnpUVEFxWmo0?=
 =?utf-8?B?dUE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b3874df-e348-458d-56a1-08dac3652013
X-MS-Exchange-CrossTenant-AuthSource: MW5PR10MB5738.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2022 21:47:12.5160
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GCJKDiCGa7h8ktWdPvl3SJN3nw8t/E6hPZdyxB02acMlN5jXzr9PeOgyjeQ2o8SHKcDvtnrmdiB8XgQNNeLHyORQHokxWITBto8575GG8/k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4377
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-10_14,2022-11-09_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 spamscore=0 malwarescore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 phishscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211100153
X-Proofpoint-GUID: eLDWsRW_o5_IRJCzCOVLCqNGjJPwAgIn
X-Proofpoint-ORIG-GUID: eLDWsRW_o5_IRJCzCOVLCqNGjJPwAgIn
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Applied with trivial changes. I cleaned up the commit message a little 
and removed a couple blank lines.

Thanks!

On 10/28/22 7:22AM, Oleg Kanatov wrote:
> jfs_mount_rw can call diUnmount and then diMount. These calls
> change the imap pointer.
> In case JFS filesystem is mounted on root (/), between these two
> calls there may be calls of function jfs_lookup().
> The jfs_lookup() function calls jfs_iget(), which, in its turn,
> calls diRead(). The latter references the imap pointer.
> That may cause diRead() to refer to a pointer "freed" in
> diUnmount().
> This commit makes calls diUnmount()/diMount() be atomic so that
> nothing will read the imap pointer until the whole remount is
> completed.
> 
> Signed-off-by: Oleg Kanatov <okanatov@gmail.com>
> ---
>   fs/jfs/jfs_imap.c  | 2 +-
>   fs/jfs/jfs_mount.c | 6 ++++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 799d3837e7c2..390cbfce391f 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -310,8 +310,8 @@ int diRead(struct inode *ip)
>   	iagno = INOTOIAG(ip->i_ino);
>   
>   	/* read the iag */
> -	imap = JFS_IP(ipimap)->i_imap;
>   	IREAD_LOCK(ipimap, RDWRLOCK_IMAP);
> +	imap = JFS_IP(ipimap)->i_imap;
>   	rc = diIAGRead(imap, iagno, &mp);
>   	IREAD_UNLOCK(ipimap);
>   	if (rc) {
> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> index 48d1f70f786c..972b9ff18723 100644
> --- a/fs/jfs/jfs_mount.c
> +++ b/fs/jfs/jfs_mount.c
> @@ -234,12 +234,18 @@ int jfs_mount_rw(struct super_block *sb, int remount)
>   
>   		truncate_inode_pages(sbi->ipimap->i_mapping, 0);
>   		truncate_inode_pages(sbi->ipbmap->i_mapping, 0);
> +
> +		IWRITE_LOCK(sbi->ipimap, RDWRLOCK_IMAP);
> +
>   		diUnmount(sbi->ipimap, 1);
>   		if ((rc = diMount(sbi->ipimap))) {
> +			IWRITE_UNLOCK(sbi->ipimap);
>   			jfs_err("jfs_mount_rw: diMount failed!");
>   			return rc;
>   		}
>   
> +		IWRITE_UNLOCK(sbi->ipimap);
> +
>   		dbUnmount(sbi->ipbmap, 1);
>   		if ((rc = dbMount(sbi->ipbmap))) {
>   			jfs_err("jfs_mount_rw: dbMount failed!");