View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000614 | Pgpool-II | Bug | public | 2020-05-26 16:07 | 2020-08-24 09:22 |
| Reporter | gregn123 | Assigned To | t-ishii | ||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | closed | Resolution | open | ||
| Product Version | 4.1.0 | ||||
| Target Version | 4.1.3 | Fixed in Version | 4.1.3 | ||
| Summary | 0000614: Invalid memory reference in sync_backend_from_watchdog() | ||||
| Description | This refers to the following code in the sync_backend_from_watchdog() function, in src/main/pgpool_main.c: if (backendStatus->primary_node_id == -1 && BACKEND_INFO(Req_info->primary_node_id).backend_status != CON_DOWN) { ereport(LOG, (errmsg("primary node:%d on master watchdog node \"%s\" seems to be quarantined", Req_info->primary_node_id, backendStatus->nodeName), errdetail("keeping the current primary"))); } else { Req_info->primary_node_id = backendStatus->primary_node_id; primary_changed = true; } During run of regression test 004.watchdog, sync_backend_from_watchdog() referenced BACKEND_INFO using a primary node id of -2, which caused memory to be referenced outside of the BACKEND_INFO. On RHEL7 for IBM Z, this always caused a crash (coredump). The problem was introduced by the following commit: https://github.com/pgpool/pgpool2/commit/3922c12c1f8efbc1b5f2e7def1e0ff921aafb989 I've attached a patch for review. | ||||
| Steps To Reproduce | regression test 004.watchdog The crash may or may not occur, depending upon whether the memory referenced by BACKEND_INFO(-2) is actually accessible or not. We didn't get a crash when running under Linux on Intel hardware (obviously the memory it referenced was accessible, but the value retrieved was essentially junk). Running under Linux on IBM Z hardware, it always crashed. | ||||
| Tags | No tags attached. | ||||
|
|
|
|
|
Thank you for the detailed analysis and the patch. -2 is the initial value for Req_info->primary_node_id, that means that the variable was never be set by the time when the crash occurs. So question is, why it was not updated and left as an initial value -2? When the regression test is executed, pgpool is running in "raw mode", which assumes that PostgreSQL is not running in streaming replication mode. If pgpool runs in raw mode, Req_info->primary_node_id will be left the initial value and will never be updated. So I think the whole code block below should only be executed in streaming replication mode. This does not mean your patch is bogus: I think your patch should be applied as well to guard against the unwanted out of band array access (I guess that could happen depending on timing). I will work on this tomorrow. if (Req_info->primary_node_id != backendStatus->primary_node_id) { ereport(LOG, (errmsg("Req_info->primary_node_id: %d processState: %d", Req_info->primary_node_id, processState))); /* Do not produce this log message if we are starting up the Pgpool-II */ if (processState != INITIALIZING) ereport(LOG, (errmsg("primary node:%d on master watchdog node \"%s\" is different from local primary node:%d", backendStatus->primary_node_id, backendStatus->nodeName, Req_info->primary_node_id))); /* * master node returns primary_node_id = -1 when the node primary * node is in quarantine state on the master. * So we will not update our primary node id when the status of current primary node * is not CON_DOWN while primary_node_id sent by master watchdong node is -1 */ if (backendStatus->primary_node_id == -1 && BACKEND_INFO(Req_info->primary_node_id).backend_status != CON_DOWN) { ereport(LOG, (errmsg("primary node:%d on master watchdog node \"%s\" seems to be quarantined", Req_info->primary_node_id, backendStatus->nodeName), errdetail("keeping the current primary"))); } else { Req_info->primary_node_id = backendStatus->primary_node_id; primary_changed = true; } } |
|
|
> I will work on this tomorrow. Done. Patch committed with slight modifications by me. Thank you for the patch! https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=cd32f6ab91d77ccdb253da5da3e2e1b6f3c933bf |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-05-26 16:07 | gregn123 | New Issue | |
| 2020-05-26 16:07 | gregn123 | File Added: 0001-Fix-invalid-memory-ref-in-sync_backend_from_watchdog.patch | |
| 2020-05-26 16:17 | t-ishii | Assigned To | => t-ishii |
| 2020-05-26 16:17 | t-ishii | Status | new => assigned |
| 2020-05-26 16:17 | t-ishii | Description Updated | |
| 2020-05-26 16:17 | t-ishii | Steps to Reproduce Updated | |
| 2020-05-26 22:44 | t-ishii | Note Added: 0003370 | |
| 2020-05-27 15:43 | t-ishii | Note Added: 0003371 | |
| 2020-05-27 15:44 | t-ishii | Status | assigned => resolved |
| 2020-05-27 16:41 | administrator | Target Version | => 4.1.3 |
| 2020-08-24 09:22 | administrator | Status | resolved => closed |
| 2020-08-24 09:22 | administrator | Fixed in Version | => 4.1.3 |
