linux-kernel - Re: [PATCH v2 1/2] staging: iio: ad9834: Correct phase range check

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] [day] [month] [year] [list]

Message-ID: <20241109144126.5efb84b6@jic23-huawei>
Date: Sat, 9 Nov 2024 14:41:26 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Zicheng Qu <quzicheng@...wei.com>, lars@...afoo.de,
 Michael.Hennerich@...log.com, gregkh@...uxfoundation.org,
 daniel.baluta@...el.com, shubhrajyoti@...com, linux-iio@...r.kernel.org,
 linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
 tanghui20@...wei.com, zhangqiao22@...wei.com, judy.chenhui@...wei.com
Subject: Re: [PATCH v2 1/2] staging: iio: ad9834: Correct phase range check

On Thu, 7 Nov 2024 13:32:42 +0300
Dan Carpenter <dan.carpenter@...aro.org> wrote:

> On Thu, Nov 07, 2024 at 01:10:14AM +0000, Zicheng Qu wrote:
> > User Perspective:
> > When a user sets the phase value, the ad9834_write_phase() is called.
> > The phase register has a 12-bit resolution, so the valid range is 0 to
> > 4095. If the phase offset value of 4096 is input, it effectively exactly
> > equals 0 in the lower 12 bits, meaning no offset.
> > 
> > Reasons for the Change:
> > 1) Original Condition (phase > BIT(AD9834_PHASE_BITS)):
> > This condition allows a phase value equal to 2^12, which is 4096.
> > However, this value exceeds the valid 12-bit range, as the maximum valid
> > phase value should be 4095.
> > 2) Modified Condition (phase >= BIT(AD9834_PHASE_BITS)):
> > Ensures that the phase value is within the valid range, preventing
> > invalid datafrom being written.
> > 
> > Impact on Subsequent Logic: st->data = cpu_to_be16(addr | phase):
> > If the phase value is 2^12, i.e., 4096 (0001 0000 0000 0000), and addr
> > is AD9834_REG_PHASE0 (1100 0000 0000 0000), then addr | phase results in
> > 1101 0000 0000 0000, occupying DB12. According to the section of WRITING
> > TO A PHASE REGISTER in the datasheet, the MSB 12 PHASE0 bits should be
> > DB11. The original condition leads to incorrect DB12 usage, which
> > contradicts the datasheet and could pose potential issues for future
> > updates if DB12 is used in such related cases.
> > 
> > Fixes: 12b9d5bf76bf ("Staging: IIO: DDS: AD9833 / AD9834 driver")
> > Cc: <stable@...r.kernel.org>
> > Signed-off-by: Zicheng Qu <quzicheng@...wei.com>
> > ---  
> 
> Fair enough.  Thanks.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@...aro.org>
Applied both patches to the fixes-togreg branch of iio.git.

Note they probably won't go upstream now until after rc1.

Thanks,

Jonathan

> 
> regards,
> dan carpenter
>