All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: linux-usb@vger.kernel.org
Cc: John Keeping <john@metanate.com>, Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Pavel Hofman <pavel.hofman@ivitera.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] usb: gadget: u_audio: fix calculations for small bInterval
Date: Tue,  4 Jan 2022 18:32:42 +0000	[thread overview]
Message-ID: <20220104183243.718258-1-john@metanate.com> (raw)

If bInterval is 1, then p_interval is 8000 and p_interval_mil is 8E9,
which is too big for a 32-bit value.  While the storage is indeed
64-bit, this value is used as the divisor in do_div() which will
truncate it into a uint32_t leading to incorrect calculated values.

Switch back to keeping the base value in struct snd_uac_chip which fits
easily into an int, meaning that the division can be done in two steps
with the divisor fitting safely into a uint32_t on both steps.

Fixes: 6fec018a7e70 ("usb: gadget: u_audio.c: Adding Playback Pitch ctl for sync playback")
Signed-off-by: John Keeping <john@metanate.com>
---
 drivers/usb/gadget/function/u_audio.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index c46400be5464..4fb05f9576a6 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -76,8 +76,8 @@ struct snd_uac_chip {
 	struct snd_pcm *pcm;
 
 	/* pre-calculated values for playback iso completion */
-	unsigned long long p_interval_mil;
 	unsigned long long p_residue_mil;
+	unsigned int p_interval;
 	unsigned int p_framesize;
 };
 
@@ -194,21 +194,24 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 		 * If there is a residue from this division, add it to the
 		 * residue accumulator.
 		 */
+		unsigned long long p_interval_mil = uac->p_interval * 1000000ULL;
+
 		pitched_rate_mil = (unsigned long long)
 				params->p_srate * prm->pitch;
 		div_result = pitched_rate_mil;
-		do_div(div_result, uac->p_interval_mil);
+		do_div(div_result, uac->p_interval);
+		do_div(div_result, 1000000);
 		frames = (unsigned int) div_result;
 
 		pr_debug("p_srate %d, pitch %d, interval_mil %llu, frames %d\n",
-				params->p_srate, prm->pitch, uac->p_interval_mil, frames);
+				params->p_srate, prm->pitch, p_interval_mil, frames);
 
 		p_pktsize = min_t(unsigned int,
 					uac->p_framesize * frames,
 					ep->maxpacket);
 
 		if (p_pktsize < ep->maxpacket) {
-			residue_frames_mil = pitched_rate_mil - frames * uac->p_interval_mil;
+			residue_frames_mil = pitched_rate_mil - frames * p_interval_mil;
 			p_pktsize_residue_mil = uac->p_framesize * residue_frames_mil;
 		} else
 			p_pktsize_residue_mil = 0;
@@ -222,11 +225,11 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 		 * size and decrease the accumulator.
 		 */
 		div_result = uac->p_residue_mil;
-		do_div(div_result, uac->p_interval_mil);
+		do_div(div_result, uac->p_interval);
+		do_div(div_result, 1000000);
 		if ((unsigned int) div_result >= uac->p_framesize) {
 			req->length += uac->p_framesize;
-			uac->p_residue_mil -= uac->p_framesize *
-					   uac->p_interval_mil;
+			uac->p_residue_mil -= uac->p_framesize * p_interval_mil;
 			pr_debug("increased req length to %d\n", req->length);
 		}
 		pr_debug("remains uac->p_residue_mil %llu\n", uac->p_residue_mil);
@@ -591,7 +594,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	unsigned int factor;
 	const struct usb_endpoint_descriptor *ep_desc;
 	int req_len, i;
-	unsigned int p_interval, p_pktsize;
+	unsigned int p_pktsize;
 
 	ep = audio_dev->in_ep;
 	prm = &uac->p_prm;
@@ -612,11 +615,10 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	/* pre-compute some values for iso_complete() */
 	uac->p_framesize = params->p_ssize *
 			    num_channels(params->p_chmask);
-	p_interval = factor / (1 << (ep_desc->bInterval - 1));
-	uac->p_interval_mil = (unsigned long long) p_interval * 1000000;
+	uac->p_interval = factor / (1 << (ep_desc->bInterval - 1));
 	p_pktsize = min_t(unsigned int,
 				uac->p_framesize *
-					(params->p_srate / p_interval),
+					(params->p_srate / uac->p_interval),
 				ep->maxpacket);
 
 	req_len = p_pktsize;
-- 
2.34.1


             reply	other threads:[~2022-01-04 18:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-04 18:32 John Keeping [this message]
2022-01-05 10:36 ` [PATCH] usb: gadget: u_audio: fix calculations for small bInterval Pavel Hofman

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=20220104183243.718258-1-john@metanate.com \
    --to=john@metanate.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=pavel.hofman@ivitera.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.