| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Junwang Zhao <zhjwpku(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix small psql slash option leaks |
| Date: | 2026-08-13 05:06:07 |
| Message-ID: | 6BD7545D-7D00-4385-BAC4-73F2C8C52BCD@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Aug 13, 2026, at 02:03, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Wed, Aug 12, 2026 at 9:01 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
>> Could psql_scan_slash_option() return the static oom_buffer when
>> initPQExpBuffer() fails while parsing a quoted option? If so, the
>> caller's free() would be invalid.
>
> Yes, you're right! On out-of-memory, psql_scan_slash_option() can return
> the static oom_buffer, so a caller that frees the returned pointer can
> trigger an invalid free. I found that psql_scan_slash_command() has
> the same kind of issue.
>
> This is separate from the leak fixed by the original patch. Existing
> callers can already free the results of these functions, so this
> should be fixed independently.
>
> Attached are three patches:
>
> - 0001 fixes the psql_scan_slash_command() case.
> - 0002 fixes the psql_scan_slash_option() case.
> - 0003 is the original leak fix, unchanged.
>
> The first two patches make, on OOM, these functions report "out of memory"
> with pg_log_error() and return NULL instead of oom_buffer, preventing
> callers from passing the static buffer to free().
>
> One limitation of 0002 is that NULL from psql_scan_slash_option() can
> still mean either "no option" or OOM. I considered adding machinery to
> distinguish the two cases and updating all callers accordingly, but that
> seemed unnecessarily large and complex for this issue.
>
> So, for now, I think returning NULL after reporting the out-of-memory
> error is a reasonable small fix, at least for the stable branches.
> Even if a caller treats the NULL as no option, the user will still see
> the out-of-memory error message output via that pg_log_error().
>
> Thoughts?
I think it’s OK. As this change is for the psql client, OOM should be rare. Even if it occurs, an error message is printed, so users will be able to see it.
My only concern was whether pg_log_error() could still print the message when OOM occurs, but after checking the code, I don’t think that is a problem.
>
> Regards,
>
> --
> Fujii Masao
> <v2-0001-Avoid-returning-oom_buffer-from-psql-slash-comman.patch><v2-0003-Fix-psql-slash-option-leaks.patch><v2-0002-Avoid-returning-oom_buffer-from-psql-slash-option.patch>
V2 LGTM.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Pavel Stehule | 2026-08-13 04:47:06 | Re: missing possibility to use alternative translated month names in to_char function |