| Message ID | 1519468495-67769-1-git-send-email-cgxu519@icloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Yan, Any objection for this? > 在 2018年2月24日，下午6:34，Chengguang Xu <cgxu519@icloud.com> 写道： > > ceph_get_cap() can return NULL, so should check return value carefully > in case of using NULL pointer unexpectedly. > > Signed-off-by: Chengguang Xu <cgxu519@icloud.com> > --- > Only compile tested. > > fs/ceph/caps.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index 6582c45..9ef40ae 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -3510,6 +3510,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex, > } else if (tsession) { > /* add placeholder for the export tagert */ > int flag = (cap == ci->i_auth_cap) ? CEPH_CAP_FLAG_AUTH : 0; > + > + if (!new_cap) > + goto out_unlock; > + > tcap = new_cap; > ceph_add_cap(inode, tsession, t_cap_id, -1, issued, 0, > t_seq - 1, t_mseq, (u64)-1, flag, &new_cap); > @@ -3764,6 +3768,11 @@ void ceph_handle_caps(struct ceph_mds_session *session, > > if (op == CEPH_CAP_OP_IMPORT) { > cap = ceph_get_cap(mdsc, NULL); > + if (!cap) { > + pr_err("%s: can't get cap\n", __func__); > + goto bad_cap; > + } > + > cap->cap_ino = vino.ino; > cap->queue_release = 1; > cap->cap_id = le64_to_cpu(h->cap_id); > @@ -3864,6 +3873,7 @@ void ceph_handle_caps(struct ceph_mds_session *session, > > bad: > pr_err("ceph_handle_caps: corrupt message\n"); > +bad_cap: > ceph_msg_dump(msg); > return; > } > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 6, 2018 at 12:22 PM, Chengguang Xu <cgxu519@icloud.com> wrote: > Hi Yan, > > Any objection for this? > Failing to record a cap from mds is serious problem. It may hang the mds. I'd like to kill the whole mount in this case. >> 在 2018年2月24日，下午6:34，Chengguang Xu <cgxu519@icloud.com> 写道： >> >> ceph_get_cap() can return NULL, so should check return value carefully >> in case of using NULL pointer unexpectedly. >> >> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> >> --- >> Only compile tested. >> >> fs/ceph/caps.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >> index 6582c45..9ef40ae 100644 >> --- a/fs/ceph/caps.c >> +++ b/fs/ceph/caps.c >> @@ -3510,6 +3510,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex, >> } else if (tsession) { >> /* add placeholder for the export tagert */ >> int flag = (cap == ci->i_auth_cap) ? CEPH_CAP_FLAG_AUTH : 0; >> + >> + if (!new_cap) >> + goto out_unlock; >> + >> tcap = new_cap; >> ceph_add_cap(inode, tsession, t_cap_id, -1, issued, 0, >> t_seq - 1, t_mseq, (u64)-1, flag, &new_cap); >> @@ -3764,6 +3768,11 @@ void ceph_handle_caps(struct ceph_mds_session *session, >> >> if (op == CEPH_CAP_OP_IMPORT) { >> cap = ceph_get_cap(mdsc, NULL); >> + if (!cap) { >> + pr_err("%s: can't get cap\n", __func__); >> + goto bad_cap; >> + } >> + >> cap->cap_ino = vino.ino; >> cap->queue_release = 1; >> cap->cap_id = le64_to_cpu(h->cap_id); >> @@ -3864,6 +3873,7 @@ void ceph_handle_caps(struct ceph_mds_session *session, >> >> bad: >> pr_err("ceph_handle_caps: corrupt message\n"); >> +bad_cap: >> ceph_msg_dump(msg); >> return; >> } >> -- >> 1.8.3.1 >> > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Sent: Tuesday, March 06, 2018 at 4:22 PM > From: "Yan, Zheng" <ukernel@gmail.com> > To: "Chengguang Xu" <cgxu519@icloud.com> > Cc: "Yan, Zheng" <zyan@redhat.com>, "Ceph Development" <ceph-devel@vger.kernel.org>, "Ilya Dryomov" <idryomov@gmail.com>, cgxu519@gmx.com > Subject: Re: [PATCH] ceph: check return value of ceph_get_cap() > > On Tue, Mar 6, 2018 at 12:22 PM, Chengguang Xu <cgxu519@icloud.com> wrote: > > Hi Yan, > > > > Any objection for this? > > > > Failing to record a cap from mds is serious problem. It may hang the > mds. I'd like to kill the whole mount in this case. How about calling panic() instead of working continuously? > > >> 在 2018年2月24日，下午6:34，Chengguang Xu <cgxu519@icloud.com> 写道： > >> > >> ceph_get_cap() can return NULL, so should check return value carefully > >> in case of using NULL pointer unexpectedly. > >> > >> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> > >> --- > >> Only compile tested. > >> > >> fs/ceph/caps.c | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > >> index 6582c45..9ef40ae 100644 > >> --- a/fs/ceph/caps.c > >> +++ b/fs/ceph/caps.c > >> @@ -3510,6 +3510,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex, > >> } else if (tsession) { > >> /* add placeholder for the export tagert */ > >> int flag = (cap == ci->i_auth_cap) ? CEPH_CAP_FLAG_AUTH : 0; > >> + > >> + if (!new_cap) > >> + goto out_unlock; > >> + > >> tcap = new_cap; > >> ceph_add_cap(inode, tsession, t_cap_id, -1, issued, 0, > >> t_seq - 1, t_mseq, (u64)-1, flag, &new_cap); > >> @@ -3764,6 +3768,11 @@ void ceph_handle_caps(struct ceph_mds_session *session, > >> > >> if (op == CEPH_CAP_OP_IMPORT) { > >> cap = ceph_get_cap(mdsc, NULL); > >> + if (!cap) { > >> + pr_err("%s: can't get cap\n", __func__); > >> + goto bad_cap; > >> + } > >> + > >> cap->cap_ino = vino.ino; > >> cap->queue_release = 1; > >> cap->cap_id = le64_to_cpu(h->cap_id); > >> @@ -3864,6 +3873,7 @@ void ceph_handle_caps(struct ceph_mds_session *session, > >> > >> bad: > >> pr_err("ceph_handle_caps: corrupt message\n"); > >> +bad_cap: > >> ceph_msg_dump(msg); > >> return; > >> } > >> -- > >> 1.8.3.1 > >> > > > > -- > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 6, 2018 at 6:25 PM, Chengguang Xu <cgxu519@gmx.com> wrote: >> Sent: Tuesday, March 06, 2018 at 4:22 PM >> From: "Yan, Zheng" <ukernel@gmail.com> >> To: "Chengguang Xu" <cgxu519@icloud.com> >> Cc: "Yan, Zheng" <zyan@redhat.com>, "Ceph Development" <ceph-devel@vger.kernel.org>, "Ilya Dryomov" <idryomov@gmail.com>, cgxu519@gmx.com >> Subject: Re: [PATCH] ceph: check return value of ceph_get_cap() >> >> On Tue, Mar 6, 2018 at 12:22 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >> > Hi Yan, >> > >> > Any objection for this? >> > >> >> Failing to record a cap from mds is serious problem. It may hang the >> mds. I'd like to kill the whole mount in this case. > > How about calling panic() instead of working continuously? > null cap will trigger oops, that's enough. > >> >> >> 在 2018年2月24日，下午6:34，Chengguang Xu <cgxu519@icloud.com> 写道： >> >> >> >> ceph_get_cap() can return NULL, so should check return value carefully >> >> in case of using NULL pointer unexpectedly. >> >> >> >> Signed-off-by: Chengguang Xu <cgxu519@icloud.com> >> >> --- >> >> Only compile tested. >> >> >> >> fs/ceph/caps.c | 10 ++++++++++ >> >> 1 file changed, 10 insertions(+) >> >> >> >> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >> >> index 6582c45..9ef40ae 100644 >> >> --- a/fs/ceph/caps.c >> >> +++ b/fs/ceph/caps.c >> >> @@ -3510,6 +3510,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex, >> >> } else if (tsession) { >> >> /* add placeholder for the export tagert */ >> >> int flag = (cap == ci->i_auth_cap) ? CEPH_CAP_FLAG_AUTH : 0; >> >> + >> >> + if (!new_cap) >> >> + goto out_unlock; >> >> + >> >> tcap = new_cap; >> >> ceph_add_cap(inode, tsession, t_cap_id, -1, issued, 0, >> >> t_seq - 1, t_mseq, (u64)-1, flag, &new_cap); >> >> @@ -3764,6 +3768,11 @@ void ceph_handle_caps(struct ceph_mds_session *session, >> >> >> >> if (op == CEPH_CAP_OP_IMPORT) { >> >> cap = ceph_get_cap(mdsc, NULL); >> >> + if (!cap) { >> >> + pr_err("%s: can't get cap\n", __func__); >> >> + goto bad_cap; >> >> + } >> >> + >> >> cap->cap_ino = vino.ino; >> >> cap->queue_release = 1; >> >> cap->cap_id = le64_to_cpu(h->cap_id); >> >> @@ -3864,6 +3873,7 @@ void ceph_handle_caps(struct ceph_mds_session *session, >> >> >> >> bad: >> >> pr_err("ceph_handle_caps: corrupt message\n"); >> >> +bad_cap: >> >> ceph_msg_dump(msg); >> >> return; >> >> } >> >> -- >> >> 1.8.3.1 >> >> >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 6582c45..9ef40ae 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -3510,6 +3510,10 @@ static void handle_cap_export(struct inode *inode, struct ceph_mds_caps *ex, } else if (tsession) { /* add placeholder for the export tagert */ int flag = (cap == ci->i_auth_cap) ? CEPH_CAP_FLAG_AUTH : 0; + + if (!new_cap) + goto out_unlock; + tcap = new_cap; ceph_add_cap(inode, tsession, t_cap_id, -1, issued, 0, t_seq - 1, t_mseq, (u64)-1, flag, &new_cap); @@ -3764,6 +3768,11 @@ void ceph_handle_caps(struct ceph_mds_session *session, if (op == CEPH_CAP_OP_IMPORT) { cap = ceph_get_cap(mdsc, NULL); + if (!cap) { + pr_err("%s: can't get cap\n", __func__); + goto bad_cap; + } + cap->cap_ino = vino.ino; cap->queue_release = 1; cap->cap_id = le64_to_cpu(h->cap_id); @@ -3864,6 +3873,7 @@ void ceph_handle_caps(struct ceph_mds_session *session, bad: pr_err("ceph_handle_caps: corrupt message\n"); +bad_cap: ceph_msg_dump(msg); return; }
ceph_get_cap() can return NULL, so should check return value carefully in case of using NULL pointer unexpectedly. Signed-off-by: Chengguang Xu <cgxu519@icloud.com> --- Only compile tested. fs/ceph/caps.c | 10 ++++++++++ 1 file changed, 10 insertions(+)