From nobody Wed Dec 17 19:06:43 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E13D8C61D99 for ; Thu, 23 Nov 2023 01:54:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232445AbjKWByb (ORCPT ); Wed, 22 Nov 2023 20:54:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229453AbjKWBy2 (ORCPT ); Wed, 22 Nov 2023 20:54:28 -0500 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 08F14B9; Wed, 22 Nov 2023 17:54:33 -0800 (PST) Received: from kwepemm000012.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4SbLg866hJzrV9n; Thu, 23 Nov 2023 09:51:00 +0800 (CST) Received: from build.huawei.com (10.175.101.6) by kwepemm000012.china.huawei.com (7.193.23.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 23 Nov 2023 09:54:30 +0800 From: Wenchao Hao To: Xiubo Li , Ilya Dryomov , Jeff Layton , Luis Henriques , CC: , Wenchao Hao Subject: [PATCH v3] ceph: quota: Fix invalid pointer access if get_quota_realm return ERR_PTR Date: Thu, 23 Nov 2023 09:53:40 +0800 Message-ID: <20231123015340.3935321-1-haowenchao2@huawei.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.175.101.6] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To kwepemm000012.china.huawei.com (7.193.23.142) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" This issue is reported by smatch that get_quota_realm() might return ERR_PTR but we did not handle it. It's not a immediate bug, while we still should address it to avoid potential bugs if get_quota_realm() is changed to return other ERR_PTR in future. Set ceph_snap_realm's pointer in get_quota_realm()'s to address this issue, the pointer would be set to NULL if get_quota_realm() failed to get struct ceph_snap_realm, so no ERR_PTR would happen any more. Signed-off-by: Wenchao Hao --- V3: - Remove redundant variable initialization in ceph_quota_is_same_realm V2: - Fix all potential invalid pointer access caused by get_quota_realm - Update commit comment and point it's not a immediate bug fs/ceph/quota.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c index 9d36c3532de1..b25906a5bfbb 100644 --- a/fs/ceph/quota.c +++ b/fs/ceph/quota.c @@ -197,10 +197,10 @@ void ceph_cleanup_quotarealms_inodes(struct ceph_mds_= client *mdsc) } =20 /* - * This function walks through the snaprealm for an inode and returns the - * ceph_snap_realm for the first snaprealm that has quotas set (max_files, + * This function walks through the snaprealm for an inode and set the + * realmp with the first snaprealm that has quotas set (max_files, * max_bytes, or any, depending on the 'which_quota' argument). If the ro= ot is - * reached, return the root ceph_snap_realm instead. + * reached, set the realmp with the root ceph_snap_realm instead. * * Note that the caller is responsible for calling ceph_put_snap_realm() o= n the * returned realm. @@ -211,10 +211,9 @@ void ceph_cleanup_quotarealms_inodes(struct ceph_mds_c= lient *mdsc) * this function will return -EAGAIN; otherwise, the snaprealms walk-throu= gh * will be restarted. */ -static struct ceph_snap_realm *get_quota_realm(struct ceph_mds_client *mds= c, - struct inode *inode, - enum quota_get_realm which_quota, - bool retry) +static int get_quota_realm(struct ceph_mds_client *mdsc, struct inode *ino= de, + enum quota_get_realm which_quota, + struct ceph_snap_realm **realmp, bool retry) { struct ceph_client *cl =3D mdsc->fsc->client; struct ceph_inode_info *ci =3D NULL; @@ -222,8 +221,10 @@ static struct ceph_snap_realm *get_quota_realm(struct = ceph_mds_client *mdsc, struct inode *in; bool has_quota; =20 + if (realmp) + *realmp =3D NULL; if (ceph_snap(inode) !=3D CEPH_NOSNAP) - return NULL; + return 0; =20 restart: realm =3D ceph_inode(inode)->i_snap_realm; @@ -250,7 +251,7 @@ static struct ceph_snap_realm *get_quota_realm(struct c= eph_mds_client *mdsc, break; ceph_put_snap_realm(mdsc, realm); if (!retry) - return ERR_PTR(-EAGAIN); + return -EAGAIN; goto restart; } =20 @@ -259,8 +260,11 @@ static struct ceph_snap_realm *get_quota_realm(struct = ceph_mds_client *mdsc, iput(in); =20 next =3D realm->parent; - if (has_quota || !next) - return realm; + if (has_quota || !next) { + if (realmp) + *realmp =3D realm; + return 0; + } =20 ceph_get_snap_realm(mdsc, next); ceph_put_snap_realm(mdsc, realm); @@ -269,7 +273,7 @@ static struct ceph_snap_realm *get_quota_realm(struct c= eph_mds_client *mdsc, if (realm) ceph_put_snap_realm(mdsc, realm); =20 - return NULL; + return 0; } =20 bool ceph_quota_is_same_realm(struct inode *old, struct inode *new) @@ -277,6 +281,7 @@ bool ceph_quota_is_same_realm(struct inode *old, struct= inode *new) struct ceph_mds_client *mdsc =3D ceph_sb_to_mdsc(old->i_sb); struct ceph_snap_realm *old_realm, *new_realm; bool is_same; + int ret; =20 restart: /* @@ -286,9 +291,9 @@ bool ceph_quota_is_same_realm(struct inode *old, struct= inode *new) * dropped and we can then restart the whole operation. */ down_read(&mdsc->snap_rwsem); - old_realm =3D get_quota_realm(mdsc, old, QUOTA_GET_ANY, true); - new_realm =3D get_quota_realm(mdsc, new, QUOTA_GET_ANY, false); - if (PTR_ERR(new_realm) =3D=3D -EAGAIN) { + get_quota_realm(mdsc, old, QUOTA_GET_ANY, &old_realm, true); + ret =3D get_quota_realm(mdsc, new, QUOTA_GET_ANY, &new_realm, false); + if (ret =3D=3D -EAGAIN) { up_read(&mdsc->snap_rwsem); if (old_realm) ceph_put_snap_realm(mdsc, old_realm); @@ -492,8 +497,8 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fs= c, struct kstatfs *buf) bool is_updated =3D false; =20 down_read(&mdsc->snap_rwsem); - realm =3D get_quota_realm(mdsc, d_inode(fsc->sb->s_root), - QUOTA_GET_MAX_BYTES, true); + get_quota_realm(mdsc, d_inode(fsc->sb->s_root), + QUOTA_GET_MAX_BYTES, &realm, true); up_read(&mdsc->snap_rwsem); if (!realm) return false; --=20 2.32.0