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-next>] [day] [month] [year] [list]
Message-Id: <1423126602-6639-1-git-send-email-hofrat@osadl.org>
Date:	Thu,  5 Feb 2015 03:56:42 -0500
From:	Nicholas Mc Guire <hofrat@...dl.org>
To:	Mauro Carvalho Chehab <mchehab@....samsung.com>
Cc:	Hans Verkuil <hans.verkuil@...co.com>,
	Wolfram Sang <wsa@...-dreams.de>, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH RFC] media: radio: handle timeouts

Add handling for timeout case.

Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---
Some error state/error information seems be get lost int the current code.
(line-numbers are from 3.19.0-rc7.

Assume that on line 827 core->write succeeds but the following 
wait_for_completion_timeout times out and the radio->irq_received condition 
is not satisfied resulting in   goto out;

827       r = core->write(core, WL1273_TUNER_MODE_SET, TUNER_MODE_AUTO_SEEK);
828       if (r)
829               goto out;
830
831       wait_for_completion_timeout(&radio->busy, msecs_to_jiffies(1000));
832       if (!(radio->irq_received & WL1273_BL_EVENT))
833               goto out;


A similar situation is at line 955 - 859 where a tiemout could occure
and the reported value would be the success value from core->write.

852       reinit_completion(&radio->busy);
853       dev_dbg(radio->dev, "%s: BUSY\n", __func__);
854
855       r = core->write(core, WL1273_TUNER_MODE_SET, TUNER_MODE_AUTO_SEEK);
856       if (r)
857               goto out;
858
859       wait_for_completion_timeout(&radio->busy, msecs_to_jiffies(1000)

the problem is that the value of r now is the "success" value from core->write
and any timeout and/or failure to detect the expected interrupt is not 
reported in 

860 out:
861       dev_dbg(radio->dev, "%s: Err: %d\n", __func__, r);
862       return r;

Should the wait_for_completion_timeout not report the timeout event by setting 
r to -ETIMEOUT ? respectively use if (!(radio->irq_received & WL1273_BL_EVENT))
to check and set -ETIMEOUT there ?

Comparing this with wl1273_fm_set_tx_freq - the below patch might be suitable 
way to handle timeout - but this needs a review by someone who knows the 
details of the driver - so this is really just a guess.

Patch was only compile tested with x86_64_defconfig + CONFIG_MEDIA_SUPPORT=m
CONFIG_MEDIA_CAMERA_SUPPORT=y, CONFIG_V4L_PLATFORM_DRIVERS=y,
CONFIG_MEDIA_RADIO_SUPPORT=y, RADIO_ADAPTER=y, CONFIG_RADIO_WL1273=m

Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)

 drivers/media/radio/radio-wl1273.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/radio-wl1273.c b/drivers/media/radio/radio-wl1273.c
index 571c7f6..6830523 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -828,9 +828,12 @@ static int wl1273_fm_set_seek(struct wl1273_device *radio,
 	if (r)
 		goto out;
 
+	/* wait for the FR IRQ */
 	wait_for_completion_timeout(&radio->busy, msecs_to_jiffies(1000));
-	if (!(radio->irq_received & WL1273_BL_EVENT))
+	if (!(radio->irq_received & WL1273_BL_EVENT)) {
+		r = -ETIMEDOUT;
 		goto out;
+	}
 
 	radio->irq_received &= ~WL1273_BL_EVENT;
 
@@ -856,7 +859,9 @@ static int wl1273_fm_set_seek(struct wl1273_device *radio,
 	if (r)
 		goto out;
 
-	wait_for_completion_timeout(&radio->busy, msecs_to_jiffies(1000));
+	/* wait for the FR IRQ */
+	if (!wait_for_completion_timeout(&radio->busy, msecs_to_jiffies(1000)))
+		r = -ETIMEDOUT;
 out:
 	dev_dbg(radio->dev, "%s: Err: %d\n", __func__, r);
 	return r;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ