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
