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: <f1d504ad-65d9-4580-a626-a98ae5d76e77@oss.nxp.com>
Date: Wed, 21 May 2025 09:05:33 +0800
From: "Ming Qian(OSS)" <ming.qian@....nxp.com>
To: Frank Li <Frank.li@....com>
Cc: mchehab@...nel.org, hverkuil-cisco@...all.nl, mirela.rabulea@....nxp.com,
 nicolas@...fresne.ca, shawnguo@...nel.org, s.hauer@...gutronix.de,
 kernel@...gutronix.de, festevam@...il.com, xiahong.bao@....com,
 eagle.zhou@....com, linux-imx@....com, imx@...ts.linux.dev,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] media: imx-jpeg: Account for data_offset when getting
 image address

Hi Frank,

On 2025/5/20 4:57, Frank Li wrote:
> On Tue, May 06, 2025 at 04:08:15PM +0800, ming.qian@....nxp.com wrote:
>> From: Ming Qian <ming.qian@....nxp.com>
>>
>> Applications may set data_offset when it refers to an output queue. So
>> driver need to account for it when getting the start address of input
>> image in the plane.
>>
>> Meanwhile the mxc-jpeg codec requires the address (plane address +
>> data_offset) to be 16-aligned.
> 
> look like it is bug fix, missed consider data_offset.
> 
> So need fix tag.
> 
> Frank
> 

Thanks for the reminder, will add it in v3.

regards,
Ming

>>
>> Signed-off-by: Ming Qian <ming.qian@....nxp.com>
>> ---
>> v2
>> - Verify the address alignment in buf_prepare()
>>
>> ---
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 47 ++++++++++++++-----
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  1 +
>>   2 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 5c17bc58181e..8681dd193033 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -598,6 +598,27 @@ static void _bswap16(u16 *a)
>>   	*a = ((*a & 0x00FF) << 8) | ((*a & 0xFF00) >> 8);
>>   }
>>
>> +static dma_addr_t mxc_jpeg_get_plane_dma_addr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> +	if (plane_no >= buf->num_planes)
>> +		return 0;
>> +	return vb2_dma_contig_plane_dma_addr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static void *mxc_jpeg_get_plane_vaddr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> +	if (plane_no >= buf->num_planes)
>> +		return NULL;
>> +	return vb2_plane_vaddr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static unsigned long mxc_jpeg_get_plane_payload(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> +	if (plane_no >= buf->num_planes)
>> +		return 0;
>> +	return vb2_get_plane_payload(buf, plane_no) - buf->planes[plane_no].data_offset;
>> +}
>> +
>>   static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>>   			  unsigned long len)
>>   {
>> @@ -610,11 +631,11 @@ static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>>   		return;
>>
>>   	for (plane_no = 0; plane_no < buf->num_planes; plane_no++) {
>> -		payload = vb2_get_plane_payload(buf, plane_no);
>> +		payload = mxc_jpeg_get_plane_payload(buf, plane_no);
>>   		if (len == 0)
>>   			len = payload;
>> -		dma_addr = vb2_dma_contig_plane_dma_addr(buf, plane_no);
>> -		vaddr = vb2_plane_vaddr(buf, plane_no);
>> +		dma_addr = mxc_jpeg_get_plane_dma_addr(buf, plane_no);
>> +		vaddr = mxc_jpeg_get_plane_vaddr(buf, plane_no);
>>   		v4l2_dbg(3, debug, &jpeg->v4l2_dev,
>>   			 "plane %d (vaddr=%p dma_addr=%x payload=%ld):",
>>   			  plane_no, vaddr, dma_addr, payload);
>> @@ -712,16 +733,15 @@ static void mxc_jpeg_addrs(struct mxc_jpeg_desc *desc,
>>   	struct mxc_jpeg_q_data *q_data;
>>
>>   	q_data = mxc_jpeg_get_q_data(ctx, raw_buf->type);
>> -	desc->buf_base0 = vb2_dma_contig_plane_dma_addr(raw_buf, 0);
>> +	desc->buf_base0 = mxc_jpeg_get_plane_dma_addr(raw_buf, 0);
>>   	desc->buf_base1 = 0;
>>   	if (img_fmt == STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV420)) {
>>   		if (raw_buf->num_planes == 2)
>> -			desc->buf_base1 = vb2_dma_contig_plane_dma_addr(raw_buf, 1);
>> +			desc->buf_base1 = mxc_jpeg_get_plane_dma_addr(raw_buf, 1);
>>   		else
>>   			desc->buf_base1 = desc->buf_base0 + q_data->sizeimage[0];
>>   	}
>> -	desc->stm_bufbase = vb2_dma_contig_plane_dma_addr(jpeg_buf, 0) +
>> -		offset;
>> +	desc->stm_bufbase = mxc_jpeg_get_plane_dma_addr(jpeg_buf, 0) + offset;
>>   }
>>
>>   static bool mxc_jpeg_is_extended_sequential(const struct mxc_jpeg_fmt *fmt)
>> @@ -1029,8 +1049,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>>   			vb2_set_plane_payload(&dst_buf->vb2_buf, 1, payload);
>>   		}
>>   		dev_dbg(dev, "Decoding finished, payload size: %ld + %ld\n",
>> -			vb2_get_plane_payload(&dst_buf->vb2_buf, 0),
>> -			vb2_get_plane_payload(&dst_buf->vb2_buf, 1));
>> +			mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 0),
>> +			mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 1));
>>   	}
>>
>>   	/* short preview of the results */
>> @@ -1889,8 +1909,8 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, struct vb2_buffer *vb)
>>   	struct mxc_jpeg_sof *psof = NULL;
>>   	struct mxc_jpeg_sos *psos = NULL;
>>   	struct mxc_jpeg_src_buf *jpeg_src_buf = vb2_to_mxc_buf(vb);
>> -	u8 *src_addr = (u8 *)vb2_plane_vaddr(vb, 0);
>> -	u32 size = vb2_get_plane_payload(vb, 0);
>> +	u8 *src_addr = (u8 *)mxc_jpeg_get_plane_vaddr(vb, 0);
>> +	u32 size = mxc_jpeg_get_plane_payload(vb, 0);
>>   	int ret;
>>
>>   	memset(&header, 0, sizeof(header));
>> @@ -2027,6 +2047,11 @@ static int mxc_jpeg_buf_prepare(struct vb2_buffer *vb)
>>   				i, vb2_plane_size(vb, i), sizeimage);
>>   			return -EINVAL;
>>   		}
>> +		if (!IS_ALIGNED(mxc_jpeg_get_plane_dma_addr(vb, i), MXC_JPEG_ADDR_ALIGNMENT)) {
>> +			dev_err(dev, "planes[%d] address is not %d aligned\n",
>> +				i, MXC_JPEG_ADDR_ALIGNMENT);
>> +			return -EINVAL;
>> +		}
>>   	}
>>   	if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type)) {
>>   		vb2_set_plane_payload(vb, 0, 0);
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> index fdde45f7e163..44e46face6d1 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> @@ -30,6 +30,7 @@
>>   #define MXC_JPEG_MAX_PLANES		2
>>   #define MXC_JPEG_PATTERN_WIDTH		128
>>   #define MXC_JPEG_PATTERN_HEIGHT		64
>> +#define MXC_JPEG_ADDR_ALIGNMENT		16
>>
>>   enum mxc_jpeg_enc_state {
>>   	MXC_JPEG_ENCODING	= 0, /* jpeg encode phase */
>>
>> base-commit: 2e79181dfc85e1347a8655ea8d8a314158155c52
>> prerequisite-patch-id: 0000000000000000000000000000000000000000
>> --
>> 2.43.0-rc1
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ