Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0d9114b

Browse files
committed
aio: Fix crash potential for pg_aios views due to late state update
pgaio_io_reclaim() reset the fields in PgAioHandle before updating the stateto IDLE or incrementing the generation. For most things that's OK, but forpg_get_aios() it is not - if it copied the PgAioHandle while fields were beingreset, we wouldn't detect that and could callpgaio_io_get_target_description() with ioh->target == PGAIO_TID_INVALID,leading to a crash.Fix this issue by incrementing the generation and state earlier, beforeresetting.Also add an assertion to pgaio_io_get_target_description() for the target tobe valid - that'd have made this case a bit easier to debug. While at it,add/update a few related assertions.Author: Alexander Lakhin <exclusion@gmail.com>Discussion:https://postgr.es/m/062daca9-dfad-4750-9da8-b13388301ad9@gmail.com
1 parent76d52e7 commit0d9114b

File tree

2 files changed

+24
-9
lines changed

2 files changed

+24
-9
lines changed

‎src/backend/storage/aio/aio.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,21 @@ pgaio_io_reclaim(PgAioHandle *ioh)
670670

671671
Assert(!ioh->resowner);
672672

673+
/*
674+
* Update generation & state first, before resetting the IO's fields,
675+
* otherwise a concurrent "viewer" could think the fields are valid, even
676+
* though they are being reset. Increment the generation first, so that
677+
* we can assert elsewhere that we never wait for an IDLE IO. While it's
678+
* a bit weird for the state to go backwards for a generation, it's OK
679+
* here, as there cannot be references to the "reborn" IO yet. Can't
680+
* update both at once, so something has to give.
681+
*/
682+
ioh->generation++;
683+
pgaio_io_update_state(ioh,PGAIO_HS_IDLE);
684+
685+
/* ensure the state update is visible before we reset fields */
686+
pg_write_barrier();
687+
673688
ioh->op=PGAIO_OP_INVALID;
674689
ioh->target=PGAIO_TID_INVALID;
675690
ioh->flags=0;
@@ -679,12 +694,6 @@ pgaio_io_reclaim(PgAioHandle *ioh)
679694
ioh->result=0;
680695
ioh->distilled_result.status=PGAIO_RS_UNKNOWN;
681696

682-
/* XXX: the barrier is probably superfluous */
683-
pg_write_barrier();
684-
ioh->generation++;
685-
686-
pgaio_io_update_state(ioh,PGAIO_HS_IDLE);
687-
688697
/*
689698
* We push the IO to the head of the idle IO list, that seems more cache
690699
* efficient in cases where only a few IOs are used.

‎src/backend/storage/aio/aio_target.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ pgaio_io_has_target(PgAioHandle *ioh)
4949
constchar*
5050
pgaio_io_get_target_name(PgAioHandle*ioh)
5151
{
52-
Assert(ioh->target >=0&&ioh->target<PGAIO_TID_COUNT);
52+
/* explicitly allow INVALID here, function used by debug messages */
53+
Assert(ioh->target >=PGAIO_TID_INVALID&&ioh->target<PGAIO_TID_COUNT);
5354

5455
returnpgaio_target_info[ioh->target]->name;
5556
}
@@ -82,6 +83,9 @@ pgaio_io_get_target_data(PgAioHandle *ioh)
8283
char*
8384
pgaio_io_get_target_description(PgAioHandle*ioh)
8485
{
86+
/* disallow INVALID, there wouldn't be a description */
87+
Assert(ioh->target>PGAIO_TID_INVALID&&ioh->target<PGAIO_TID_COUNT);
88+
8589
returnpgaio_target_info[ioh->target]->describe_identity(&ioh->target_data);
8690
}
8791

@@ -98,6 +102,8 @@ pgaio_io_get_target_description(PgAioHandle *ioh)
98102
bool
99103
pgaio_io_can_reopen(PgAioHandle*ioh)
100104
{
105+
Assert(ioh->target>PGAIO_TID_INVALID&&ioh->target<PGAIO_TID_COUNT);
106+
101107
returnpgaio_target_info[ioh->target]->reopen!=NULL;
102108
}
103109

@@ -109,8 +115,8 @@ pgaio_io_can_reopen(PgAioHandle *ioh)
109115
void
110116
pgaio_io_reopen(PgAioHandle*ioh)
111117
{
112-
Assert(ioh->target >=0&&ioh->target<PGAIO_TID_COUNT);
113-
Assert(ioh->op >=0&&ioh->op<PGAIO_OP_COUNT);
118+
Assert(ioh->target>PGAIO_TID_INVALID&&ioh->target<PGAIO_TID_COUNT);
119+
Assert(ioh->op>PGAIO_OP_INVALID&&ioh->op<PGAIO_OP_COUNT);
114120

115121
pgaio_target_info[ioh->target]->reopen(ioh);
116122
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp