Postgres Pro
Downloads
Home   >   mailing lists

Thread: pnstrdup considered armed and dangerous

pnstrdup considered armed and dangerous

From
Andres Freund
Date:
Hi,

A colleage of me just wrote innocent looking code like       char *shardRelationName = pnstrdup(relationName,
NAMEDATALEN);
which is at the moment wrong if relationName isn't preallocated to
NAMEDATALEN size.

/** pnstrdup*        Like pstrdup(), but append null byte to a*        not-necessarily-null-terminated input string.*/
char *
pnstrdup(const char *in, Size len)
{char       *out = palloc(len + 1);
memcpy(out, in, len);out[len] = '\0';return out;
}

isn't that a somewhat weird behaviour / implementation? Not really like
strndup(), which one might believe to be analoguous...

Greetings,

Andres Freund

Re: pnstrdup considered armed and dangerous

From
Robert Haas
Date:
On Mon, Oct 3, 2016 at 5:55 PM, Andres Freund <andres@anarazel.de> wrote:
> /*
>  * pnstrdup
>  *              Like pstrdup(), but append null byte to a
>  *              not-necessarily-null-terminated input string.
>  */
> char *
> pnstrdup(const char *in, Size len)
> {
>         char       *out = palloc(len + 1);
>
>         memcpy(out, in, len);
>         out[len] = '\0';
>         return out;
> }
>
> isn't that a somewhat weird behaviour / implementation? Not really like
> strndup(), which one might believe to be analoguous...

Yikes!

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: pnstrdup considered armed and dangerous

From
Geoff Winkless
Date:
On 3 October 2016 at 22:55, Andres Freund <andres@anarazel.de> wrote:
> A colleage of me just wrote innocent looking code like
>         char *shardRelationName = pnstrdup(relationName, NAMEDATALEN);
> which is at the moment wrong if relationName isn't preallocated to
> NAMEDATALEN size.
[snip]
> isn't that a somewhat weird behaviour / implementation? Not really like
> strndup(), which one might believe to be analoguous...

Well I wouldn't say it's wrong, exactly. It might produce a segfault
if relationName[NAMEDATALEN] is outside readable memory for the
process, but otherwise it will behave as defined.

Geoff

Re: pnstrdup considered armed and dangerous

From
Geoff Winkless
Date:
On 4 October 2016 at 14:12, Geoff Winkless <pgsqladmin@geoff.dj> wrote:
> Well I wouldn't say it's wrong, exactly. It might produce a segfault
> if relationName[NAMEDATALEN] is outside readable memory for the
> process, but otherwise it will behave as defined.

Finger slippage. Of course I meant

... if relationName[NAMEDATALEN-1] is outside...

Geoff

Re: [HACKERS] pnstrdup considered armed and dangerous

From
Andres Freund
Date:
On 2016-10-03 14:55:24 -0700, Andres Freund wrote:
> Hi,
> 
> A colleage of me just wrote innocent looking code like
>         char *shardRelationName = pnstrdup(relationName, NAMEDATALEN);
> which is at the moment wrong if relationName isn't preallocated to
> NAMEDATALEN size.
> 
> /*
>  * pnstrdup
>  *        Like pstrdup(), but append null byte to a
>  *        not-necessarily-null-terminated input string.
>  */
> char *
> pnstrdup(const char *in, Size len)
> {
>     char       *out = palloc(len + 1);
> 
>     memcpy(out, in, len);
>     out[len] = '\0';
>     return out;
> }
> 
> isn't that a somewhat weird behaviour / implementation? Not really like
> strndup(), which one might believe to be analoguous...

I've since hit this bug again. To fix it, you'd need strnlen. The lack
of which I'd also independently hit twice.  So here's a patch adding
pg_strnlen and using that to fix pnstrdup.

Greetings,

Andres Freund

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