linux-kernel - Re: [PATCH v18 05/15] media: amphion: implement vpu core communication based on mailbox

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: <20220309122356.GD2592@kili>
Date:   Wed, 9 Mar 2022 15:23:56 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Ming Qian <ming.qian@....com>
Cc:     mchehab@...nel.org, shawnguo@...nel.org, robh+dt@...nel.org,
        s.hauer@...gutronix.de, hverkuil-cisco@...all.nl,
        kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
        aisheng.dong@....com, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v18 05/15] media: amphion: implement vpu core
 communication based on mailbox

On Thu, Feb 24, 2022 at 11:10:03AM +0800, Ming Qian wrote:
> +static bool vpu_inst_receive_msg(struct vpu_inst *inst, struct vpu_rpc_event *pkt)
> +{
> +	u32 bytes = sizeof(struct vpu_rpc_event_header);
> +	u32 ret;
> +
> +	memset(pkt, 0, sizeof(*pkt));
> +	if (kfifo_len(&inst->msg_fifo) < bytes)
> +		return false;
> +
> +	ret = kfifo_out(&inst->msg_fifo, pkt, bytes);
> +	if (ret != bytes)
> +		return false;
> +
> +	if (pkt->hdr.num > 0) {
> +		bytes = pkt->hdr.num * sizeof(u32);

Imagine if we had declared "bytes" as unsigned long, then on 64 bit
systems we wouldn't have to worry about integer overflows here.  The u32
type has a lot of negatives and none of the positives that unsigned long
has.

> +		ret = kfifo_out(&inst->msg_fifo, pkt->data, bytes);
> +		if (ret != bytes)
> +			return false;
> +	}
> +
> +	return true;
> +}

[ Snip ]

> +void vpu_msg_delayed_work(struct work_struct *work)
> +{
> +	struct vpu_core *core;
> +	struct delayed_work *dwork;
> +	u32 bytes = sizeof(bytes);

LOL...  You got jokes.

> +	u32 i;
> +
> +	if (!work)
> +		return;
> +
> +	dwork = to_delayed_work(work);
> +	core = container_of(dwork, struct vpu_core, msg_delayed_work);
> +	if (kfifo_len(&core->msg_fifo) >= bytes)
> +		vpu_core_run_msg_work(core);
> +
> +	bytes = sizeof(struct vpu_rpc_event_header);
> +	for (i = 0; i < core->supported_instance_count; i++) {
> +		struct vpu_inst *inst = vpu_core_find_instance(core, i);
> +
> +		if (!inst)
> +			continue;
> +
> +		if (inst->workqueue && kfifo_len(&inst->msg_fifo) >= bytes)
> +			queue_work(inst->workqueue, &inst->msg_work);
> +
> +		vpu_inst_put(inst);
> +	}
> +}

regards,
dan carpenter