Skip to content

Commit 0f4cf64

Browse files
wenchao-haoidryomov
authored andcommitted
ceph: fix invalid pointer access if get_quota_realm return ERR_PTR
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. [ xiubli: minor code style clean up ] Signed-off-by: Wenchao Hao <haowenchao2@huawei.com> Reviewed-by: Xiubo Li <xiubli@redhat.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
1 parent b36b033 commit 0f4cf64

File tree

1 file changed

+22
-17
lines changed

1 file changed

+22
-17
lines changed

fs/ceph/quota.c

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,10 @@ void ceph_cleanup_quotarealms_inodes(struct ceph_mds_client *mdsc)
197197
}
198198

199199
/*
200-
* This function walks through the snaprealm for an inode and returns the
201-
* ceph_snap_realm for the first snaprealm that has quotas set (max_files,
200+
* This function walks through the snaprealm for an inode and set the
201+
* realmp with the first snaprealm that has quotas set (max_files,
202202
* max_bytes, or any, depending on the 'which_quota' argument). If the root is
203-
* reached, return the root ceph_snap_realm instead.
203+
* reached, set the realmp with the root ceph_snap_realm instead.
204204
*
205205
* Note that the caller is responsible for calling ceph_put_snap_realm() on the
206206
* returned realm.
@@ -211,19 +211,20 @@ void ceph_cleanup_quotarealms_inodes(struct ceph_mds_client *mdsc)
211211
* this function will return -EAGAIN; otherwise, the snaprealms walk-through
212212
* will be restarted.
213213
*/
214-
static struct ceph_snap_realm *get_quota_realm(struct ceph_mds_client *mdsc,
215-
struct inode *inode,
216-
enum quota_get_realm which_quota,
217-
bool retry)
214+
static int get_quota_realm(struct ceph_mds_client *mdsc, struct inode *inode,
215+
enum quota_get_realm which_quota,
216+
struct ceph_snap_realm **realmp, bool retry)
218217
{
219218
struct ceph_client *cl = mdsc->fsc->client;
220219
struct ceph_inode_info *ci = NULL;
221220
struct ceph_snap_realm *realm, *next;
222221
struct inode *in;
223222
bool has_quota;
224223

224+
if (realmp)
225+
*realmp = NULL;
225226
if (ceph_snap(inode) != CEPH_NOSNAP)
226-
return NULL;
227+
return 0;
227228

228229
restart:
229230
realm = ceph_inode(inode)->i_snap_realm;
@@ -250,7 +251,7 @@ static struct ceph_snap_realm *get_quota_realm(struct ceph_mds_client *mdsc,
250251
break;
251252
ceph_put_snap_realm(mdsc, realm);
252253
if (!retry)
253-
return ERR_PTR(-EAGAIN);
254+
return -EAGAIN;
254255
goto restart;
255256
}
256257

@@ -259,8 +260,11 @@ static struct ceph_snap_realm *get_quota_realm(struct ceph_mds_client *mdsc,
259260
iput(in);
260261

261262
next = realm->parent;
262-
if (has_quota || !next)
263-
return realm;
263+
if (has_quota || !next) {
264+
if (realmp)
265+
*realmp = realm;
266+
return 0;
267+
}
264268

265269
ceph_get_snap_realm(mdsc, next);
266270
ceph_put_snap_realm(mdsc, realm);
@@ -269,14 +273,15 @@ static struct ceph_snap_realm *get_quota_realm(struct ceph_mds_client *mdsc,
269273
if (realm)
270274
ceph_put_snap_realm(mdsc, realm);
271275

272-
return NULL;
276+
return 0;
273277
}
274278

275279
bool ceph_quota_is_same_realm(struct inode *old, struct inode *new)
276280
{
277281
struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(old->i_sb);
278282
struct ceph_snap_realm *old_realm, *new_realm;
279283
bool is_same;
284+
int ret;
280285

281286
restart:
282287
/*
@@ -286,9 +291,9 @@ bool ceph_quota_is_same_realm(struct inode *old, struct inode *new)
286291
* dropped and we can then restart the whole operation.
287292
*/
288293
down_read(&mdsc->snap_rwsem);
289-
old_realm = get_quota_realm(mdsc, old, QUOTA_GET_ANY, true);
290-
new_realm = get_quota_realm(mdsc, new, QUOTA_GET_ANY, false);
291-
if (PTR_ERR(new_realm) == -EAGAIN) {
294+
get_quota_realm(mdsc, old, QUOTA_GET_ANY, &old_realm, true);
295+
ret = get_quota_realm(mdsc, new, QUOTA_GET_ANY, &new_realm, false);
296+
if (ret == -EAGAIN) {
292297
up_read(&mdsc->snap_rwsem);
293298
if (old_realm)
294299
ceph_put_snap_realm(mdsc, old_realm);
@@ -492,8 +497,8 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf)
492497
bool is_updated = false;
493498

494499
down_read(&mdsc->snap_rwsem);
495-
realm = get_quota_realm(mdsc, d_inode(fsc->sb->s_root),
496-
QUOTA_GET_MAX_BYTES, true);
500+
get_quota_realm(mdsc, d_inode(fsc->sb->s_root), QUOTA_GET_MAX_BYTES,
501+
&realm, true);
497502
up_read(&mdsc->snap_rwsem);
498503
if (!realm)
499504
return false;

0 commit comments

Comments
 (0)