Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: Enzo Matsumiya <ematsumiya@suse.de>
To: linux-cifs@vger.kernel.org
Cc: smfrench@gmail.com, pc@cjr.nz, ronniesahlberg@gmail.com,
	nspmangalore@gmail.com
Subject: [PATCH] cifs: check if mid was deleted in async read callback
Date: Sun, 18 Sep 2022 20:54:42 -0300	[thread overview]
Message-ID: <20220918235442.2981-1-ematsumiya@suse.de> (raw)

There's a race when cifs_readv_receive() might dequeue the mid,
and mid->callback(), called from demultiplex thread, will try to
access it to verify the signature before the mid is actually
released/deleted.

Currently the signature verification fails, but the verification
shouldn't have happened at all because the mid was deleted because
of an error, and hence not really supposed to be passed to
->callback(). There are no further errors because the mid is
effectivelly gone by the end of the callback.

This patch checks if the mid doesn't have the MID_DELETED flag set (by
dequeue_mid()) right before trying to verify the signature. According to
my tests, trying to check it earlier, e.g. after the ->receive() call in
cifs_demultiplex_thread, will fail most of the time as dequeue_mid()
might not have been called yet.

This behaviour can be seen in xfstests generic/465, for example, where
mids with STATUS_END_OF_FILE (-ENODATA) are dequeued and supposed to be
discarded, but instead have their signature computed, but mismatched.

Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
 fs/cifs/cifssmb.c | 2 +-
 fs/cifs/smb2pdu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index addf3fc62aef..116f6afe33c6 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1308,7 +1308,7 @@ cifs_readv_callback(struct mid_q_entry *mid)
 	switch (mid->mid_state) {
 	case MID_RESPONSE_RECEIVED:
 		/* result already set, check signature */
-		if (server->sign) {
+		if (server->sign && !(mid->mid_flags & MID_DELETED)) {
 			int rc = 0;
 
 			rc = cifs_verify_signature(&rqst, server,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 5da0b596c8a0..394996c4f729 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4136,7 +4136,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
 		credits.value = le16_to_cpu(shdr->CreditRequest);
 		credits.instance = server->reconnect_instance;
 		/* result already set, check signature */
-		if (server->sign && !mid->decrypted) {
+		if (server->sign && !mid->decrypted && !(mid->mid_flags & MID_DELETED)) {
 			int rc;
 
 			rc = smb2_verify_signature(&rqst, server);
-- 
2.35.3


             reply	other threads:[~2022-09-18 23:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-18 23:54 Enzo Matsumiya [this message]
2022-09-19  1:37 ` [PATCH] cifs: check if mid was deleted in async read callback ronnie sahlberg
2022-09-19 14:44 ` Paulo Alcantara
2022-09-20  4:15   ` Steve French
2022-09-20 20:44     ` Enzo Matsumiya
2022-09-20 20:49     ` Tom Talpey
2022-09-21 14:10       ` Enzo Matsumiya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220918235442.2981-1-ematsumiya@suse.de \
    --to=ematsumiya@suse.de \
    --cc=linux-cifs@vger.kernel.org \
    --cc=nspmangalore@gmail.com \
    --cc=pc@cjr.nz \
    --cc=ronniesahlberg@gmail.com \
    --cc=smfrench@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox