pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: sockaddr_un.sun_len vs. reality
Date: Mon, 14 Feb 2022 01:18:48 -0500
Message-ID: <2781112.1644819528@sss.pgh.pa.us> (raw)

For a very long time, the AIX 7.1 buildfarm members (sungazer and tern)
have complained about

ip.c:236:17: warning: conversion from 'long unsigned int' to 'uchar_t' {aka 'unsigned char'} changes value from '1025' to '1' [-Woverflow]

I'd ignored this as not being very interesting, but I got motivated to
recheck it today.  The warning is coming from

#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
	unp->sun_len = sizeof(struct sockaddr_un);
#endif

so we can infer that the sun_len field is unsigned char and the
size of struct sockaddr_un doesn't fit.

Poking around a bit, I find that sun_len exists on most BSD-ish
platforms and it seems to be unsigned char (almost?) everywhere.
But on other platforms sizeof(struct sockaddr_un) is only a bit
over 100, so there's not an overflow problem.

It's not real clear that there's any problem on AIX either;
given that the regression tests pass, I strongly suspect that
nothing is paying attention to the sun_len field.  But if we're
going to fill it in at all, we should probably try to fill it
in with something less misleading than "1".

Googling finds that this seems to be a sore spot for various
people, eg [1] mentions the existence of the SUN_LEN() macro
on some platforms and then says why you shouldn't use it.

I'm leaning to adding a compile-time clamp on the value,
that is

	unp->sun_len = Min(sizeof(struct sockaddr_un),
	                   (1 << (sizeof(unp->sun_len) * 8)) - 1);

(This might need a little bit of refinement if sun_len
could be as wide as int, but in any case it should still
reduce to a compile-time constant.)

Or we could use something involving strlen(unp->sun_path), perhaps
trying to use SUN_LEN() if it exists.  But that implies expending
extra run-time cycles for strlen(), and I've seen no indication
that there's any value here except suppressing a compiler warning.

Thoughts?

			regards, tom lane

[1] http://mail-index.netbsd.org/tech-net/2006/10/11/0008.html





view thread (9+ messages)  latest in thread

Message-ID: <2781112.1644819528@sss.pgh.pa.us>
Permalink:  ../2781112.1644819528@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/2781112.1644819528@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
  Subject: Re: sockaddr_un.sun_len vs. reality
  In-Reply-To: <2781112.1644819528@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox