linux-kernel - Re: [PATCH v9 1/1] Add support for IPMB driver

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]

Date:   Sun, 19 May 2019 16:02:31 +0200
From:   Wolfram Sang <wsa@...-dreams.de>
To:     Asmaa Mnebhi <Asmaa@...lanox.com>
Cc:     minyard@....org, vadimp@...lanox.com, michaelsh@...lanox.com,
        rdunlap@...radead.org, linux-kernel@...r.kernel.org,
        linux-i2c@...r.kernel.org
Subject: Re: [PATCH v9 1/1] Add support for IPMB driver


> +static int receive_ipmb_request(struct ipmb_dev *ipmb_dev,
> +				bool non_blocking,
> +				struct ipmb_msg *ipmb_request)
> +{
> +	struct ipmb_request_elem *queue_elem;
> +	unsigned long flags;
> +	int res;
> +
> +	spin_lock_irqsave(&ipmb_dev->lock, flags);
> +
> +	while (!atomic_read(&ipmb_dev->request_queue_len)) {

Am I overlooking something? Why are you protecting an atomic_read with a
spinlock?

> +		spin_unlock_irqrestore(&ipmb_dev->lock, flags);
> +
> +		if (non_blocking)
> +			return -EAGAIN;
> +
> +		res = wait_event_interruptible(ipmb_dev->wait_queue,
> +				atomic_read(&ipmb_dev->request_queue_len));
> +		if (res)
> +			return res;
> +
> +		spin_lock_irqsave(&ipmb_dev->lock, flags);
> +	}

...

> +	rq_sa = msg[RQ_SA_8BIT_IDX] >> 1;
> +	netf_rq_lun = msg[NETFN_LUN_IDX];
> +	/*
> +	 * subtract rq_sa and netf_rq_lun from the length of the msg passed to
> +	 * i2c_smbus_write_block_data_local
> +	 */
> +	msg_len = msg[IPMB_MSG_LEN_IDX] - SMBUS_MSG_HEADER_LENGTH;
> +
> +	strcpy(rq_client.name, "ipmb_requester");
> +	rq_client.adapter = ipmb_dev->client->adapter;
> +	rq_client.flags = ipmb_dev->client->flags;
> +	rq_client.addr = rq_sa;

Is it possible to determine in a race-free way if rq_sa (which came from
userspace AFAIU) is really the address from which the request came in
(again if I understood all this correctly)?


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)