linux-kernel - Re: [PATCH v7 08/13] ksmbd: add smb3 engine part 1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210805110219.GJ22532@kadam>
Date:   Thu, 5 Aug 2021 14:02:20 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Namjae Jeon <namjae.jeon@...sung.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-cifs@...r.kernel.org,
        linux-cifsd-devel@...ts.sourceforge.net, aurelien.aptel@...il.com,
        sandeen@...deen.net, willy@...radead.org, hch@...radead.org,
        senozhatsky@...omium.org, christian@...uner.io,
        viro@...iv.linux.org.uk, ronniesahlberg@...il.com, hch@....de,
        metze@...ba.org, smfrench@...il.com, hyc.lee@...il.com,
        Steve French <stfrench@...rosoft.com>
Subject: Re: [PATCH v7 08/13] ksmbd: add smb3 engine part 1

On Thu, Aug 05, 2021 at 03:05:41PM +0900, Namjae Jeon wrote:
> +/**
> + * check_session_id() - check for valid session id in smb header
> + * @conn:	connection instance
> + * @id:		session id from smb header
> + *
> + * Return:      1 if valid session id, otherwise 0
> + */
> +static inline int check_session_id(struct ksmbd_conn *conn, u64 id)

Make this bool.  Same for all the is_* functions.

> +{
> +	struct ksmbd_session *sess;
> +
> +	if (id == 0 || id == -1)
> +		return 0;
> +
> +	sess = ksmbd_session_lookup_all(conn, id);
> +	if (sess)
> +		return 1;
> +	pr_err("Invalid user session id: %llu\n", id);
> +	return 0;
> +}
> +
> +struct channel *lookup_chann_list(struct ksmbd_session *sess, struct ksmbd_conn *conn)
> +{
> +	struct channel *chann;
> +
> +	list_for_each_entry(chann, &sess->ksmbd_chann_list, chann_list) {
> +		if (chann->conn == conn)
> +			return chann;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * smb2_get_ksmbd_tcon() - get tree connection information for a tree id
> + * @work:	smb work
> + *
> + * Return:      matching tree connection on success, otherwise error

This documentation seems out of date.

> + */
> +int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
> +{
> +	struct smb2_hdr *req_hdr = work->request_buf;
> +	int tree_id;
> +
> +	work->tcon = NULL;
> +	if (work->conn->ops->get_cmd_val(work) == SMB2_TREE_CONNECT_HE ||
> +	    work->conn->ops->get_cmd_val(work) ==  SMB2_CANCEL_HE ||
> +	    work->conn->ops->get_cmd_val(work) ==  SMB2_LOGOFF_HE) {
> +		ksmbd_debug(SMB, "skip to check tree connect request\n");
> +		return 0;
> +	}
> +
> +	if (xa_empty(&work->sess->tree_conns)) {
> +		ksmbd_debug(SMB, "NO tree connected\n");
> +		return -1;

Better to return -EINVAL.

> +	}
> +
> +	tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
> +	work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
> +	if (!work->tcon) {
> +		pr_err("Invalid tid %d\n", tree_id);
> +		return -1;
> +	}
> +
> +	return 1;
> +}

regards,
dan carpenter