Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Sukrut Bellary <sukrut.bellary@linux.com>,
	Abel Vesa <abel.vesa@linaro.org>,
	Amol Maheshwari <amahesh@qti.qualcomm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] misc: fastrpc: Fix double free of 'buf' in error path
Date: Fri, 19 May 2023 13:58:10 +0300	[thread overview]
Message-ID: <fa0e9d9d-6362-456b-87f7-990ccf7e8930@kili.mountain> (raw)
In-Reply-To: <b0115d7d-d15a-4948-8726-09a8b37f3f36@kili.mountain>

This is unrelated but I was looking through the driver and I notice
a bunch of code doing:

grep 'return ret ?' drivers/firmware/ -R

	return ret ? : res.result[0];

"ret" here is a kernel error code, and res.result[0] is a firmware
error code.  Mixing error codes is a dangerous thing.  I was reviewing
some of the callers and the firmware error code gets passed quite far
back into the kernel to where we would only expect kernel error codes.

Presumably the firmware is returning positive error codes?  To be honest,
I am just guessing.  It's better to convert custom error codes to kernel
error codes as soon as possible.  I am just guessing.  Sukrut, do you
think you could take a look?  If the callers do not differentiate
between negative kernel error codes and positive custom error codes then
probably just do:

	if (res.result[0])
		ret = -EIO; // -EINVAL?
	return ret;

Also there are a couple places which do:

	return ret ? false : !!res.result[0];

Here true means success and false means failure.  So the !! converts
a firmware error code to true when it should be false so that's a bug.
Quadruple negatives are confusing...  It should be:

	if (ret || res.result[0])
		return false;
	return true;

regards,
dan carpenter


  parent reply	other threads:[~2023-05-19 10:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-18 10:08 [PATCH] misc: fastrpc: Fix double free of 'buf' in error path Sukrut Bellary
2023-05-18 10:55 ` Dan Carpenter
2023-05-19  2:45   ` Sukrut Bellary
2023-05-19  4:16     ` Dan Carpenter
2023-05-19  6:12       ` Sukrut Bellary
2023-05-19  9:52   ` Srinivas Kandagatla
2023-05-19 10:22     ` Dan Carpenter
2023-05-19 10:39       ` Srinivas Kandagatla
2023-05-19 22:57         ` Sukrut Bellary
2023-06-01  4:45           ` Sukrut Bellary
2023-06-01  7:00             ` Dan Carpenter
2023-06-01 19:09               ` Sukrut Bellary
2023-05-19 10:58       ` Dan Carpenter [this message]
2023-05-19 23:39         ` Sukrut Bellary

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fa0e9d9d-6362-456b-87f7-990ccf7e8930@kili.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=abel.vesa@linaro.org \
    --cc=amahesh@qti.qualcomm.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=sukrut.bellary@linux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox