| Message ID | 20221014201454.4456-1-pc@cjr.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: fix double-fault crash during ntlmssp | expand |
On 10/14, Paulo Alcantara wrote: >The crash occurred because we were calling memzero_explicit() on an >already freed sess_data::iov[1] (ntlmsspblob) in sess_free_buffer(). > >Fix this by not calling memzero_explicit() on sess_data::iov[1] as >it's already by handled by callers. > >Fixes: d867d4ae29c7 ("cifs: replace kfree() with kfree_sensitive() for sensitive data") >Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Good catch! Thanks. Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de> >--- > fs/cifs/sess.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > >diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c >index f1c3c6d9146c..d33a27c1af4e 100644 >--- a/fs/cifs/sess.c >+++ b/fs/cifs/sess.c >@@ -1213,16 +1213,18 @@ sess_alloc_buffer(struct sess_data *sess_data, int wct) > static void > sess_free_buffer(struct sess_data *sess_data) > { >- int i; >+ struct kvec *iov = sess_data->iov; > >- /* zero the session data before freeing, as it might contain sensitive info (keys, etc) */ >- for (i = 0; i < 3; i++) >- if (sess_data->iov[i].iov_base) >- memzero_explicit(sess_data->iov[i].iov_base, sess_data->iov[i].iov_len); >+ /* >+ * Zero the session data before freeing, as it might contain sensitive info (keys, etc). >+ * Note that iov[1] is already freed by caller. >+ */ >+ if (sess_data->buf0_type != CIFS_NO_BUFFER && iov[0].iov_base) >+ memzero_explicit(iov[0].iov_base, iov[0].iov_len); > >- free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base); >+ free_rsp_buf(sess_data->buf0_type, iov[0].iov_base); > sess_data->buf0_type = CIFS_NO_BUFFER; >- kfree(sess_data->iov[2].iov_base); >+ kfree_sensitive(iov[2].iov_base); > } > > static int >-- >2.37.3 >
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index f1c3c6d9146c..d33a27c1af4e 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -1213,16 +1213,18 @@ sess_alloc_buffer(struct sess_data *sess_data, int wct) static void sess_free_buffer(struct sess_data *sess_data) { - int i; + struct kvec *iov = sess_data->iov; - /* zero the session data before freeing, as it might contain sensitive info (keys, etc) */ - for (i = 0; i < 3; i++) - if (sess_data->iov[i].iov_base) - memzero_explicit(sess_data->iov[i].iov_base, sess_data->iov[i].iov_len); + /* + * Zero the session data before freeing, as it might contain sensitive info (keys, etc). + * Note that iov[1] is already freed by caller. + */ + if (sess_data->buf0_type != CIFS_NO_BUFFER && iov[0].iov_base) + memzero_explicit(iov[0].iov_base, iov[0].iov_len); - free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base); + free_rsp_buf(sess_data->buf0_type, iov[0].iov_base); sess_data->buf0_type = CIFS_NO_BUFFER; - kfree(sess_data->iov[2].iov_base); + kfree_sensitive(iov[2].iov_base); } static int
The crash occurred because we were calling memzero_explicit() on an already freed sess_data::iov[1] (ntlmsspblob) in sess_free_buffer(). Fix this by not calling memzero_explicit() on sess_data::iov[1] as it's already by handled by callers. Fixes: d867d4ae29c7 ("cifs: replace kfree() with kfree_sensitive() for sensitive data") Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/sess.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)