FIX #7693: concurrent task execution: fixed dereference of NULL #7694
Conversation
ProjectMutilation
changed the title
| if (!task) | ||
| { | ||
| ereport(ERROR, (errmsg("task is invalid"))); | ||
| } |
There was a problem hiding this comment.
You message is not very helpful.
See around line 300 a similar pattern, here patched:
if (!task)
{
ereport(ERROR, (errmsg("no task found with taskid: %ld", handleEntry->taskid)));
}
Also I suggest to move your patch one line up (before the assignment).
There was a problem hiding this comment.
I wonder if the error message should not be more strong: 'unexpected missing task id: ...;'
Maybe an Assert() given "task" is always expected to be set....
There was a problem hiding this comment.
ereport(ERROR, ...) doesn't return control
There was a problem hiding this comment.
I'm unsure what Citus guidelines are here but I don't think task is ever supposed to be NULL in this function.
That's why maybe an Assert() and not an ereport(ERROR)...
|
|
||
| if (!task || task->status == BACKGROUND_TASK_STATUS_CANCELLING) | ||
| if (task->status == BACKGROUND_TASK_STATUS_CANCELLING) |
There was a problem hiding this comment.
that's a bit of a trouble, in the "else" part too it depends on task being set.
In the function TaskConcurrentCancelCheck() the pointer "task" was utilized after checking against NULL, which can lead to dereference of the null pointer. To avoid the problem, added a separate handling of the case when the pointer is null with an interruption of execution. Fixes: 1f8675d("nonblocking concurrent task execution via background workers") Signed-off-by: Maksim Korotkov <m.korotkov@postgrespro.ru>
ProjectMutilation
force-pushed
the
main
branch
from
e51e314
to
73205b1
Compare
In the function TaskConcurrentCancelCheck() the pointer "task" was utilized after checking against NULL, which can lead to dereference of the null pointer.
To avoid the problem, added a separate handling of the case when the pointer is null with an interruption of execution.
Fixes: 1f8675d("nonblocking concurrent task execution via background workers")