Re: BUG #14929: Unchecked AllocateDir() return value in restoreTwoPhaseData()

On 2017/11/27 18:31, bianpan2016(at)163(dot)com wrote:
> The following bug has been logged on the website:
>
> Bug reference: 14929
> Logged by: Pan Bian
> Email address: bianpan2016(at)163(dot)com
> PostgreSQL version: 10.1
> Operating system: Linux
> Description:
>
> File: src/backend/access/transam/twophase.c
> Function: restoreTwoPhaseData
> Line: 1738
>
> AllocateDir() will return a NULL pointer if it fails to open the specified
> directory. However, in function restoreTwoPhaseData(), its return value is
> not checked. This may result in a NULL pointer dereference when trying to
> free it (see line 1759).
>
> For your convenience, I copy and paste related codes as follows:
>
> 1732 void
> 1733 restoreTwoPhaseData(void)
> 1734 {
> 1735 DIR *cldir;
> 1736 struct dirent *clde;
> 1737
> 1738 cldir = AllocateDir(TWOPHASE_DIR);
> 1739 LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
> 1740 while ((clde = ReadDir(cldir, TWOPHASE_DIR)) != NULL)
> 1741 {
> ...
> 1758 LWLockRelease(TwoPhaseStateLock);
> 1759 FreeDir(cldir);
> 1760 }

Thanks for the report.

It seems like a good idea to check cldir for NULL before freeing. Please
find attached a patch to implement the same.

Thanks,
Amit