diff mbox

ceph: check return value of ceph_get_cap()

Message ID 1519468495-67769-1-git-send-email-cgxu519@icloud.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chengguang Xu Feb. 24, 2018, 10:34 a.m. UTC
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(+)

Comments

Chengguang Xu March 6, 2018, 4:22 a.m. UTC | #1
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
Yan, Zheng March 6, 2018, 8:22 a.m. UTC | #2
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
Chengguang Xu March 6, 2018, 10:25 a.m. UTC | #3
> 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
Yan, Zheng March 6, 2018, 11:42 a.m. UTC | #4
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 mbox

Patch

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;
 }