public inbox for linux-cifs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: namjae.jeon@samsung.com
Cc: linux-cifs@vger.kernel.org
Subject: [bug report] cifsd: add server-side procedures for SMB3
Date: Thu, 8 Jul 2021 14:30:46 +0300	[thread overview]
Message-ID: <YObh5kJIG4LoHkFx@mwanda> (raw)

Hello Namjae Jeon,

The patch e2f34481b24d: "cifsd: add server-side procedures for SMB3"
from Mar 16, 2021, leads to the following static checker warning:

	fs/ksmbd/smb2pdu.c:2329 smb2_create_sd_buffer()
	warn: 'context' is an error pointer or valid

fs/ksmbd/smb2pdu.c
  2317  static int smb2_create_sd_buffer(struct ksmbd_work *work,
  2318                                   struct smb2_create_req *req,
  2319                                   struct path *path)
  2320  {
  2321          struct create_context *context;
  2322          int rc = -ENOENT;
  2323  
  2324          if (!req->CreateContextsOffset)
  2325                  return rc;
  2326  
  2327          /* Parse SD BUFFER create contexts */
  2328          context = smb2_find_context_vals(req, SMB2_CREATE_SD_BUFFER);

The comments for smb2_find_context_vals() says that it returns NULL on
error but really it never returns NULL.

When a function returns both error pointers and NULL, then NULL means
the feature has been deliberately disabled.  Or another use might be
if "p = get_next();" and get_next() will return -ENOMEM if there is an
allocation error but NULL when there aren't any more items.  In other
words NULL is a sort of success.

It's better to always write error handling instead of success handling
because normally the error handling is shorter (cleanup and return an
error code) but the success case path is more involved.  Also it results
in everything being less indented.  Also preserve the error code.

	if (IS_ERR(context))
		return PTR_ERR(context);

	ksmbd_debug(SMB, "Set ACLs using SMB2_CREATE_SD_BUFFER context\n");

  2329          if (context && !IS_ERR(context)) {
  2330                  struct create_sd_buf_req *sd_buf;
  2331  
  2332                  ksmbd_debug(SMB,
  2333                              "Set ACLs using SMB2_CREATE_SD_BUFFER context\n");
  2334                  sd_buf = (struct create_sd_buf_req *)context;
  2335                  rc = set_info_sec(work->conn, work->tcon,
  2336                                    path, &sd_buf->ntsd,
  2337                                    le32_to_cpu(sd_buf->ccontext.DataLength), true);
  2338          }
  2339  
  2340          return rc;
  2341  }

regards,
dan carpenter

             reply	other threads:[~2021-07-08 11:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-08 11:30 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-11-30 11:54 [bug report] cifsd: add server-side procedures for SMB3 Dan Carpenter
2021-11-30 23:59 ` Hyunchul Lee
2021-12-01  1:57 ` Namjae Jeon
2023-05-26 11:56 Dan Carpenter
2023-05-26 14:38 ` Namjae Jeon
2024-07-19 23:55 Dan Carpenter

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=YObh5kJIG4LoHkFx@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=namjae.jeon@samsung.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