AioCtl Shared Memory size fix

Started by Madhukar .7 months ago4 messageshackers
Jump to latest
#1Madhukar .
madhukarprasad@google.com

Hi Folks,

While reviewing AIO code, we found an issue with AioCtlShmemSize function.
The function was not accounting for io_handles which is used post shared
memory alloc in AioShmemInit. Please find a patch to account for
io_handles member of PgAioCtl.

The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles)
in AioCtlShmemSize.

Thanks,
Madhukar

Attachments:

patch.diffapplication/octet-stream; name=patch.diffDownload+1-5
#2Matthias van de Meent
boekewurm+postgres@gmail.com
In reply to: Madhukar . (#1)
Re: AioCtl Shared Memory size fix

On Mon, 15 Sept 2025 at 13:33, Madhukar . <madhukarprasad@google.com> wrote:

Hi Folks,

While reviewing AIO code, we found an issue with AioCtlShmemSize function. The function was not accounting for io_handles which is used post shared memory alloc in AioShmemInit.

Good catch.

Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.

Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.

LGTM.

Kind regards,

Matthias van de Meent
Databricks

#3Michael Paquier
michael@paquier.xyz
In reply to: Matthias van de Meent (#2)
Re: AioCtl Shared Memory size fix

On Mon, Sep 15, 2025 at 02:06:03PM +0200, Matthias van de Meent wrote:

Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.

I would bet on a FLEXIBLE_ARRAY_MEMBER from a previous version..

Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.

LGTM.

Yep, will fix. Thanks for the report, the patch and the review, to
both of you.
--
Michael

#4Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#3)
Re: AioCtl Shared Memory size fix

Hi,

On 2025-09-16 15:50:20 +0900, Michael Paquier wrote:

On Mon, Sep 15, 2025 at 02:06:03PM +0200, Matthias van de Meent wrote:

Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.

I would bet on a FLEXIBLE_ARRAY_MEMBER from a previous version..

Indeed. I don't remember for sure why I changed it, but I think it may have
been to make the different allocations more visible in pg_shmem_allocations.

Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.

LGTM.

Yep, will fix. Thanks for the report, the patch and the review, to
both of you.

Thanks for finding and fixing!

Greetings,

Andres Freund