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: <333c54f3-3d4b-4038-94a9-ce8396b452ce@suswa.mountain>
Date: Thu, 7 Nov 2024 13:32:42 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Zicheng Qu <quzicheng@...wei.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
	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, 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>

regards,
dan carpenter