linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] cifsd: add server-side procedures for SMB3
@ 2021-07-08 11:30 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2021-07-08 11:30 UTC (permalink / raw)
  To: namjae.jeon; +Cc: linux-cifs

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-07-08 11:31 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 11:30 [bug report] cifsd: add server-side procedures for SMB3 Dan Carpenter

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox