Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
In-Reply-To: <20210805110219.GJ22532@kadam>
References: <20210805060546.3268-1-namjae.jeon@samsung.com>
 <CGME20210805061600epcas1p13ca76c1e21f317f9f3f52860a70a241e@epcas1p1.samsung.com>
 <20210805060546.3268-9-namjae.jeon@samsung.com> <20210805110219.GJ22532@kadam>
From:   Namjae Jeon <linkinjeon@kernel.org>
Date:   Fri, 6 Aug 2021 06:59:30 +0900
Message-ID: <CAKYAXd8y_QPr+9oYqyGGtnunHBXKXi6Qr-YNM=sAsDSvr6r-LQ@mail.gmail.com>
Subject: Re: [Linux-cifsd-devel] [PATCH v7 08/13] ksmbd: add smb3 engine part 1
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     Namjae Jeon <namjae.jeon@samsung.com>, linux-cifs@vger.kernel.org,
        aurelien.aptel@gmail.com, linux-cifsd-devel@lists.sourceforge.net,
        sandeen@sandeen.net, linux-kernel@vger.kernel.org,
        willy@infradead.org, hch@infradead.org, senozhatsky@chromium.org,
        viro@zeniv.linux.org.uk, ronniesahlberg@gmail.com,
        linux-fsdevel@vger.kernel.org,
        Steve French <stfrench@microsoft.com>, hch@lst.de,
        christian@brauner.io
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Dan,

2021-08-05 20:02 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
> 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.
Okay, I will fix it on next version.
>
>> +{
>> +	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.
Okay.
>
>> + */
>> +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.
Okay, Will fix it.

Thanks for your review!
>
>> +	}
>> +
>> +	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
>
>
> _______________________________________________
> Linux-cifsd-devel mailing list
> Linux-cifsd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-cifsd-devel
>