[PATCH] readlink missing nul-termination in pg_rewind

Started by Abhijit Menon-Senover 10 years ago4 messages
#1Abhijit Menon-Sen
ams@2ndQuadrant.com
1 attachment(s)

This is just something I noticed in passing. (I did a quick check of all
the other uses of readlink in the source, and they do get this right.)

-- Abhijit

P.S. Also in passing, I note that pg_rewind will follow links under any
directory anywhere named pg_tblspc (which probably doesn't matter), and
does not follow pg_xlog if it's a symlink (which probably does). If you
want, I can submit a trivial patch for the latter.

Attachments:

0001-readlink-doesn-t-nul-terminate-the-buffer-so-we-have.patchtext/x-diff; charset=us-asciiDownload
>From d1e5cbea21bb916253bce2ad189deb1924864508 Mon Sep 17 00:00:00 2001
From: Abhijit Menon-Sen <ams@2ndQuadrant.com>
Date: Thu, 28 May 2015 21:03:50 +0530
Subject: readlink() doesn't nul-terminate the buffer, so we have to

---
 src/bin/pg_rewind/copy_fetch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 9e317a2..4a7150b 100644
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -126,6 +126,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 						 fullpath);
 			}
 
+			link_target[len] = '\0';
 			callback(path, FILE_TYPE_SYMLINK, 0, link_target);
 
 			/*
-- 
1.9.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Abhijit Menon-Sen (#1)
Re: [PATCH] readlink missing nul-termination in pg_rewind

Abhijit Menon-Sen <ams@2ndQuadrant.com> writes:

This is just something I noticed in passing. (I did a quick check of all
the other uses of readlink in the source, and they do get this right.)

There's more random inconsistency than just this. I think we should
standardize on the coding exhibited at, eg, basebackup.c:1023ff, which
positively ensures that it won't scribble on random memory if the
call returns an unexpected value. Will fix.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Abhijit Menon-Sen (#1)
Re: [PATCH] readlink missing nul-termination in pg_rewind

Abhijit Menon-Sen <ams@2ndQuadrant.com> writes:

P.S. Also in passing, I note that pg_rewind will follow links under any
directory anywhere named pg_tblspc (which probably doesn't matter), and
does not follow pg_xlog if it's a symlink (which probably does). If you
want, I can submit a trivial patch for the latter.

As far as that goes, I think it does look at the whole parentpath, which
means it would not be fooled by sub-subdirectories named pg_tblspc.
A bigger problem is that whoever coded this forgot that parentpath could
be null, which I blame on the lack of an API specification for the
function.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#3)
Re: [PATCH] readlink missing nul-termination in pg_rewind

On Fri, May 29, 2015 at 1:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Abhijit Menon-Sen <ams@2ndQuadrant.com> writes:

P.S. Also in passing, I note that pg_rewind will follow links under any
directory anywhere named pg_tblspc (which probably doesn't matter), and
does not follow pg_xlog if it's a symlink (which probably does). If you
want, I can submit a trivial patch for the latter.

As far as that goes, I think it does look at the whole parentpath, which
means it would not be fooled by sub-subdirectories named pg_tblspc.
A bigger problem is that whoever coded this forgot that parentpath could
be null, which I blame on the lack of an API specification for the
function.

Oh, thanks for pushing a fix for that. It was missed during the review...
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers