an interesting bug, regarding sman (low level C stuff)

50 views
Skip to first unread message

oldk1331

unread,
Jul 26, 2020, 6:00:23 AM7/26/20
to fricas...@googlegroups.com
Hi all,

Today suddenly I can't launch FriCAS, it
shows "*** buffer overflow detected ***:
terminated Aborted".

After some debugging, I found that sman
calls function "open_server" in
sockio-c.c#929:

strcpy(server[1].addr.u_addr.sa_data, name);

"sa_data" is char[14], and "name" is
"/tmp/.i"+getpid(), so on my system PID is
over a million, causing buffer overflow
in strcpy.

I wonder what's the best way to solve this
issue, and other C-string buffer overflow
in our code base.

Best wishes,
- Qian

Waldek Hebisch

unread,
Jul 28, 2020, 10:22:01 PM7/28/20
to fricas...@googlegroups.com
I am not sure about best way. Pragmatically, something
like:

-- a/src/include/com.h
+++ b/src/include/com.h
@@ -70,6 +70,7 @@ typedef struct {
union {
struct sockaddr u_addr;
struct sockaddr_in i_addr;
+ char pad[32];
} addr;
char *host_name; /* name of foreign host if type == AF_INET */
} Sock;

should work. On Linux theoreticaly cleaner would be to use
'sockaddr_un' instead of 'sockaddr' (and include 'sys/un.h').
But Windows needs different declarations. Also, in 'sockio-c.c'
theorticaly it would be safer to write:

(struct sockaddr *)(&server[1].addr)

instead of '&server[1].addr.u_addr'.

--
Waldek Hebisch

oldk1331

unread,
Aug 8, 2020, 4:59:41 AM8/8/20
to fricas...@googlegroups.com
Why do you think Windows needs different
declarations? Aren't we using cygwin/mingw?

BTW, what's our support status for windows?
Cygwin or mingw or WSL? I tried to build with
Msys2 today, but it's really a bad experience,
very slow, I gave up after 2 hours.


should work.  On Linux theoreticaly cleaner would be to use
'sockaddr_un' instead of 'sockaddr' (and include 'sys/un.h').
But Windows needs different declarations.  Also, in 'sockio-c.c'
theorticaly it would be safer to write:

 (struct sockaddr *)(&server[1].addr)

instead of '&server[1].addr.u_addr'.

--
                              Waldek Hebisch

--
You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fricas-devel/20200729022152.GA38001%40math.uni.wroc.pl.

Ralf Hemmecke

unread,
Aug 8, 2020, 6:10:40 AM8/8/20
to fricas...@googlegroups.com
> Cygwin or mingw or WSL?

What I have heard about WSL2, then it shouldn't be a big problem to
build FriCAS there. Or is is because of the X requirement?
Anyway, if I were interesedt in Windows support, I focus on WSL2.

Ralf

Waldek Hebisch

unread,
Aug 8, 2020, 9:02:22 AM8/8/20
to fricas...@googlegroups.com
On Sat, Aug 08, 2020 at 04:59:27PM +0800, oldk1331 wrote:
> Why do you think Windows needs different
> declarations? Aren't we using cygwin/mingw?

For that issue Cygwin, WSL and mingw are really
different platforms. WSL emulates Linux, so
it should be enough to treat it as Linux. Mingw
uses native Windows calls and AFAIK that is
different from Linux. In particular AFAIK
instead of Unix domain sockets Windows has
"local" sockets which offer similar functionality,
but require somewhat different code. But
for deeper information one should look into
Microsoft documentation.
>
> BTW, what's our support status for windows?
> Cygwin or mingw or WSL? I tried to build with
> Msys2 today, but it's really a bad experience,
> very slow, I gave up after 2 hours.
>

Concerning Windows status, I do not have Windows,
I know what other folks tell me. At various
times I received information about sucessful
builds on Windows, most on the list, some
private. In most cases information was rather
sparse, lacking details. Around 2007 FriCAS
used to build with Windows GCL, this was without
sman, hyperdoc and graphics, so really no need
for sockets. In this period Gabriel Dos Reis
fixed some issues with Windows sockets, they
build and probably worked OK. Later GCL support
on Windows got broken (apparently GCL could not
correctly compile rather simple Lisp code and
nobody investigated deeper). For some time
using Clisp and Cygwin gave us slow, but
fully functional FriCAS on Windows. But now
functionality that we need is no longer present
in Cygwin Clisp, so this no longer works (in
principle, if somebody managed to install old
Cygwin, then FriCAS should still build). In
principle Cygwin ECL should work, but I do not
remember if anybody had success with this
(and I remeber folks having problems with
building Cygwin ECL).

Concerning recent state, message were on the
list. In particular, beside your info above there
were message by Gregory Vanuxem in April.

Could you say what exacly you tried? And have you
any idea what was so slow? In particular,
at what stage build was when you interrupted it?

BTW. Some info I have is rather old. But I am
trying to keep old things working. As I wrote,
on Windows I depend on info from others. Few
days ago I got old Linux machine running. Now
I can say that current trunk builds with no
problem on 32-bit Linux from about 2008 using
sbcl 1.0.16.

--
Waldek Hebisch

oldk1331

unread,
Aug 9, 2020, 8:03:24 AM8/9/20
to fricas...@googlegroups.com


On Sat, Aug 8, 2020, 9:02 PM Waldek Hebisch <heb...@math.uni.wroc.pl> wrote:
On Sat, Aug 08, 2020 at 04:59:27PM +0800, oldk1331 wrote:
> Why do you think Windows needs different
> declarations? Aren't we using cygwin/mingw?

For that issue Cygwin, WSL and mingw are really
different platforms.  WSL emulates Linux, so
it should be enough to treat it as Linux.  Mingw
uses native Windows calls and AFAIK that is
different from Linux.  In particular AFAIK
instead of Unix domain sockets Windows has
"local" sockets which offer similar functionality,
but require somewhat different code.  But
for deeper information one should look into
Microsoft documentation.

I mean, if I can build fricas in mingw with this
patch (sockaddr_un) successfully, then can this
patch be merged?  Aka which platform should
I test for breakage?


Could you say what exacly you tried?  And have you
any idea what was so slow?  In particular,
at what stage build was when you interrupted it?

I'm using msys2.  The shell is extremely slow.
Probably due to anti-virus software (I can't
disable it, long story).  I gave up at ./configure,
it's 100x slower than Linux. I might try again in
VM some other time.


BTW.  Some info I have is rather old.  But I am
trying to keep old things working.  As I wrote,
on Windows I depend on info from others.  Few
days ago I got old Linux machine running.  Now
I can say that current trunk builds with no
problem on 32-bit Linux from about 2008 using
sbcl 1.0.16.

--
                              Waldek Hebisch

Well, sometimes "keep old things working"
hinders progress...

Waldek Hebisch

unread,
Aug 9, 2020, 9:15:07 AM8/9/20
to fricas...@googlegroups.com
On Sun, Aug 09, 2020 at 08:03:09PM +0800, oldk1331 wrote:
> On Sat, Aug 8, 2020, 9:02 PM Waldek Hebisch <heb...@math.uni.wroc.pl>
> wrote:
>
> > On Sat, Aug 08, 2020 at 04:59:27PM +0800, oldk1331 wrote:
> > > Why do you think Windows needs different
> > > declarations? Aren't we using cygwin/mingw?
> >
> > For that issue Cygwin, WSL and mingw are really
> > different platforms. WSL emulates Linux, so
> > it should be enough to treat it as Linux. Mingw
> > uses native Windows calls and AFAIK that is
> > different from Linux. In particular AFAIK
> > instead of Unix domain sockets Windows has
> > "local" sockets which offer similar functionality,
> > but require somewhat different code. But
> > for deeper information one should look into
> > Microsoft documentation.
> >
>
> I mean, if I can build fricas in mingw with this
> patch (sockaddr_un) successfully, then can this
> patch be merged? Aka which platform should
> I test for breakage?

What you mean by "this patch". The patch I posted
(adding unused field) should not break anything and
should fix problem that you reported.

In ideal world we should test on all supported systems.
Practically, sockaddr_un is pretty standard Unix things,
so once we get if working on Linux (that is there is
no crude errors) what remains is Windows. As I wrote,
WSL is not a problem, and if Mingw works I would
assume that there are no extra problem with Cygwin.
However, IIUC 32-bit Mingw has differences compared
to 64-bit one, so we should test on both.

Now, comming to "progress", AFAICS the patch I posted
(adding unused field) should allow us to move on with
minimal effort. ATM potential problem with this
patch look rather theortical. However, if problem
would actually show up, then we would be forced to
do deeper (say sockaddr_un) change.

To put it differently, I am trying to avoid breaking
changes, but if I have to choose between having
unfixed bug on Linux and untested code on Windows,
I would choose fixing known bug now (and handling
possible unknown breakage later).

OTOH if I have somewhat unclean code that I expect to
work on all platforms, I prefer this code compared to
cleaner looking but potentialy unportable code.

> > Could you say what exacly you tried? And have you
> > any idea what was so slow? In particular,
> > at what stage build was when you interrupted it?
> >
>
> I'm using msys2. The shell is extremely slow.
> Probably due to anti-virus software (I can't
> disable it, long story). I gave up at ./configure,
> it's 100x slower than Linux. I might try again in
> VM some other time.

I see. Long time to run 'configure' is known problem on
Windows (but 2 hours seems excessive).


--
Waldek Hebisch

oldk1331

unread,
Aug 9, 2020, 11:38:49 PM8/9/20
to fricas...@googlegroups.com

> I mean, if I can build fricas in mingw with this
> patch (sockaddr_un) successfully, then can this
> patch be merged?  Aka which platform should
> I test for breakage?

What you mean by "this patch".  The patch I posted
(adding unused field) should not break anything and
should fix problem that you reported.


I consider "adding unused field" a hack and I
prefer sockaddr_un.

In ideal world we should test on all supported systems.
Practically, sockaddr_un is pretty standard Unix things,
so once we get if working on Linux (that is there is
no crude errors) what remains is Windows.  As I wrote,
WSL is not a problem, and if Mingw works I would
assume that there are no extra problem with Cygwin.
However, IIUC 32-bit Mingw has differences compared
to 64-bit one, so we should test on both.


Ok, I'll try to test sockaddr_un on mingw32
and mingw64 then.

BTW, win10 recently supports AF_UNIX natively.
Still on mingw I guess it translates the code somehow.

P.S, on mingw we don't have graphics, so the
only purpose of sman is to provide line editing (clef)?

oldk1331

unread,
Aug 19, 2020, 4:15:55 AM8/19/20
to fricas...@googlegroups.com
On 8/10/20 11:38 AM, oldk1331 wrote:
> Ok, I'll try to test sockaddr_un on mingw32
> and mingw64 then.

I tested in mingw64 on windows, apparently
we use winsock2.h and there's no direct
support for sockaddr_un.

Let me explore other options.

- Qian

oldk1331

unread,
Aug 22, 2020, 10:41:57 AM8/22/20
to fricas...@googlegroups.com
Okay, I take a different approach and try
to workaround this problem in function
"make_server_name":

diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
index 30e0eba5..a168129f 100644
--- a/src/lib/sockio-c.c
+++ b/src/lib/sockio-c.c
@@ -857,9 +857,21 @@
int
make_server_name(char *name,char * base)
{
+ /* Try to fit the contents of 'name' into char[14],
+ which is Sock::addr::u_addr::sa_data.
+ Normally 'base' is "/tmp/.x", so that leaves room
+ for 6 chars to put 'spad_server_number'/'SPADNUM',
+ which is the PID of sman. So use hexadecimal when
+ the PID is over 10^6.
+ */
char *num;
+ int pid;
if (spad_server_number != -1) {
- sprintf(name, "%s%d", base, spad_server_number);
+ if (spad_server_number < 1000000){
+ sprintf(name, "%s%d", base, spad_server_number);
+ } else {
+ sprintf(name, "%s%x", base, spad_server_number);
+ }
return 0;
}
num = getenv("SPADNUM");
@@ -869,7 +881,12 @@ make_server_name(char *name,char * base)
*/
return -1;
}
- sprintf(name, "%s%s", base, num);
+ pid = atoi(num);
+ if (pid < 1000000) {
+ sprintf(name, "%s%s", base, num);
+ } else {
+ sprintf(name, "%s%x", base, pid);
+ }
return 0;
make_server_name.patch

Waldek Hebisch

unread,
Aug 23, 2020, 8:01:52 AM8/23/20
to fricas...@googlegroups.com
On Sat, Aug 22, 2020 at 10:41:50PM +0800, oldk1331 wrote:
> Okay, I take a different approach and try
> to workaround this problem in function
> "make_server_name":
>
> diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
> index 30e0eba5..a168129f 100644
> --- a/src/lib/sockio-c.c
> +++ b/src/lib/sockio-c.c
> @@ -857,9 +857,21 @@
> int
> make_server_name(char *name,char * base)
> {
> + /* Try to fit the contents of 'name' into char[14],
> + which is Sock::addr::u_addr::sa_data.
> + Normally 'base' is "/tmp/.x", so that leaves room
> + for 6 chars to put 'spad_server_number'/'SPADNUM',
> + which is the PID of sman. So use hexadecimal when
> + the PID is over 10^6.
> + */

Sorry, this is problematic:

1) 800000 hexadecimal is 8388608, so two different numbers give
the same name. The whole point of using PID is to make
sure there are no accidental collisions
2) Hexadecimal buys us only factor of about 16 in magnitude.
Wait few years and this will be not enough (or may be
insufficient even now). In padding hack I made sure that
we have place for 64-bit PID.

One may have objections to padding hack, but practicaly it
is more likely to work in long term than the proposal above.

--
Waldek Hebisch

oldk1331

unread,
Aug 23, 2020, 9:44:01 AM8/23/20
to fricas...@googlegroups.com
How about encoding PID with 0-9a-zA-Z or some
other hash method?

If you agree with this approach, I can get to implement it.

--
You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.

oldk1331

unread,
Aug 23, 2020, 10:05:34 AM8/23/20
to fricas...@googlegroups.com
Actually we can stick to hexadecimal for pid
less than 1 million as well.

Current possible max pid is defined by PID_MAX_LIMIT in kernel,
which is 2^22, around 4 million.

--
You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.

Waldek Hebisch

unread,
Aug 23, 2020, 12:35:53 PM8/23/20
to fricas...@googlegroups.com
On Sun, Aug 23, 2020 at 10:04:29PM +0800, oldk1331 wrote:
> Actually we can stick to hexadecimal for pid
> less than 1 million as well.
>
> Current possible max pid is defined by PID_MAX_LIMIT in kernel,
> which is 2^22, around 4 million.

I would not count too much on this limit. I have heard that
currently 32GB modules are resonably priced. With such
modules cheap 128 GB home machine is possible. With
multiprocessor (server) board 512GB should be easily
available. Such machines may support several million
processes. While demand for large number of true
processes is probably low, IIUC Linux allocated thread
identifiers from the same pool. At least some folks
tried to run with really large number of threads. So
I would expect that some people will increase the limit...

--
Waldek Hebisch

Waldek Hebisch

unread,
Aug 23, 2020, 12:47:18 PM8/23/20
to fricas...@googlegroups.com
On Sun, Aug 23, 2020 at 09:43:48PM +0800, oldk1331 wrote:
> How about encoding PID with 0-9a-zA-Z or some
> other hash method?
>
> If you agree with this approach, I can get to implement it.

For Linux base-64 could work. But if such things is supposed
to serve as Windows filename, then we are limited to
something like base-36. 6 digit base-36 number can express
all nonegative 32-bit ints, so for few years this will
do.

Looking in longer run, almost surely 32-bit PID will be
replaced by larger range, so the problem will be back...

--
Waldek Hebisch

oldk1331

unread,
Aug 26, 2020, 6:08:49 AM8/26/20
to fricas...@googlegroups.com
With 512G memory and 4 million processes, so each process
can get 128K memory? Besides, process creation is expensive,
it's never a good idea in practice to have so many process.

The linux kernel seems to indicate that each core can
support 1k processes, so 4M limit seems pretty
reasonable in a few decades.

Thread identifiers is different, see comment below.

So I believe my following patch is relatively simple
to address the original bug, and it is future-proof
for at least 20 years, and it's easily extendable
beyond that.

- Qian

diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
index 30e0eba5..e9d8a02e 100644
--- a/src/lib/sockio-c.c
+++ b/src/lib/sockio-c.c
@@ -857,9 +857,17 @@
int
make_server_name(char *name,char * base)
{
+ /* Try to fit the contents of 'name' into char[14],
+ which is Sock::addr::u_addr::sa_data.
+ Normally 'base' is "/tmp/.x", so that leaves room
+ for 6 chars to put 'spad_server_number'/'SPADNUM',
+ which is the PID of sman. So use hexadecimal to
+ represent PID.
+ */
char *num;
+ int pid;
if (spad_server_number != -1) {
- sprintf(name, "%s%d", base, spad_server_number);
+ sprintf(name, "%s%x", base, spad_server_number);
return 0;
}
num = getenv("SPADNUM");
@@ -869,7 +877,8 @@
*/
return -1;
}
- sprintf(name, "%s%s", base, num);
+ pid = atoi(num);
+ sprintf(name, "%s%x", base, pid);
return 0;
}


=====include/linux/threads.h========
/*
* A maximum of 4 million PIDs should be enough for a while.
* [NOTE: PID/TIDs are limited to 2^30 ~= 1 billion, see FUTEX_TID_MASK.]
*/
#define PID_MAX_LIMIT (CONFIG_BASE_SMALL ? PAGE_SIZE * 8 : \
(sizeof(long) > 4 ? 4 * 1024 * 1024 : PID_MAX_DEFAULT))

/*
* Define a minimum number of pids per cpu. Heuristically based
* on original pid max of 32k for 32 cpus. Also, increase the
* minimum settable value for pid_max on the running system based
* on similar defaults. See kernel/pid.c:pidmap_init() for details.
*/



Waldek Hebisch

unread,
Aug 28, 2020, 9:35:35 AM8/28/20
to fricas...@googlegroups.com
On Wed, Aug 26, 2020 at 06:08:42PM +0800, oldk1331 wrote:
> On 8/24/20 12:35 AM, Waldek Hebisch wrote:
> > On Sun, Aug 23, 2020 at 10:04:29PM +0800, oldk1331 wrote:
> >> Actually we can stick to hexadecimal for pid
> >> less than 1 million as well.
> >>
> >> Current possible max pid is defined by PID_MAX_LIMIT in kernel,
> >> which is 2^22, around 4 million.
> >
> > I would not count too much on this limit. I have heard that
> > currently 32GB modules are resonably priced. With such
> > modules cheap 128 GB home machine is possible. With
> > multiprocessor (server) board 512GB should be easily
> > available. Such machines may support several million
> > processes. While demand for large number of true
> > processes is probably low, IIUC Linux allocated thread
> > identifiers from the same pool. At least some folks
> > tried to run with really large number of threads. So
> > I would expect that some people will increase the limit...
> >
>
> With 512G memory and 4 million processes, so each process
> can get 128K memory? Besides, process creation is expensive,
> it's never a good idea in practice to have so many process.

128K memory is plenty for small process: my measurement indicate
that 32K is enough (100K used to be recommended amount of
memory per process).

> The linux kernel seems to indicate that each core can
> support 1k processes, so 4M limit seems pretty
> reasonable in a few decades.
>
> Thread identifiers is different, see comment below.

Sorry, all that kernel snippend says it that if you
want more than 2^30 PIDs you need to do serious
changes. Remember, Linux has source and you can
change limit in the source. In the past limit
was much smaller, I needed more than limit so
I simply changed number in source and it worked fine.

More to the point, the limit is quite arbitrary. It make
some sense to have such limit in kernel. But in
our case there is really no need for limit: Unix domain
sockets worked for years with longer names. And while
the hack I proposed is not nice, it is similar to
hacks that are probably in use in similar contexts:
C compiler have to tolerate access beyond declared
size to struct because lot of C code uses it. So
I see no point of going in direction you propose.

--
Waldek Hebisch

oldk1331

unread,
Aug 28, 2020, 11:24:06 AM8/28/20
to fricas...@googlegroups.com
On 8/28/20 9:35 PM, Waldek Hebisch wrote:
> the hack I proposed is not nice, it is similar to
> hacks that are probably in use in similar contexts:
> C compiler have to tolerate access beyond declared
> size to struct because lot of C code uses it. So
> I see no point of going in direction you propose.
>

OK, I agree with your explanation now, please commit
your solution.

- Qian

oldk1331

unread,
Aug 30, 2020, 8:39:40 AM8/30/20
to fricas...@googlegroups.com
Hi Waldek,

Your commit e68c2a3e doesn't work out of box on my system:

The gcc or glibc on my system is hardened (might be true
for newer gcc/glibc on other systems as well), so it
actually checks for buffer overflow in 'strcpy', so the
error message I described in the first mail still presents,
so we need to change sockio-c.c#929 to

strncpy(server[1].addr.u_addr.sa_data, name, ??);

I'm not sure about the struct layout here, which number
should be put there?

- Qian

Waldek Hebisch

unread,
Aug 30, 2020, 9:51:21 AM8/30/20
to fricas...@googlegroups.com
On Sun, Aug 30, 2020 at 08:39:32PM +0800, oldk1331 wrote:
>
> Hi Waldek,
>
> Your commit e68c2a3e doesn't work out of box on my system:
>
> The gcc or glibc on my system is hardened (might be true
> for newer gcc/glibc on other systems as well), so it
> actually checks for buffer overflow in 'strcpy', so the
> error message I described in the first mail still presents,
> so we need to change sockio-c.c#929 to
>
> strncpy(server[1].addr.u_addr.sa_data, name, ??);
>
> I'm not sure about the struct layout here, which number
> should be put there?

30. However, I think that attached patch is better.
--
Waldek Hebisch
cp.diff

oldk1331

unread,
Aug 30, 2020, 11:59:01 PM8/30/20
to fricas...@googlegroups.com
OK, I tested it and found 2 issues:

1. you need to change "open_server" as well.
2. the memset destroys the first 2 bits of the addr struct,
causing failure to "bind".

(BTW, isn't strncpy theoretically safer?)

Waldek Hebisch

unread,
Sep 7, 2020, 8:24:18 AM9/7/20
to fricas...@googlegroups.com
What about the attached patch. It skips 'memset' which seem
to be not needed at all and cleans up relevant parts.

>
> (BTW, isn't strncpy theoretically safer?)

No. gcc may add the same checks to 'strncpy' as to 'strcpy'.
In general 'strncpy' has serious problems and is only
marginaly better than 'strcpy'. In out case I do not
think that 'strncpy' buys us anything.

--
Waldek Hebisch

oldk1331

unread,
Sep 9, 2020, 6:08:38 AM9/9/20
to fricas...@googlegroups.com

> What about the attached patch. It skips 'memset' which seem
> to be not needed at all and cleans up relevant parts.

Hi Waldek, I'm afraid you forget to include the attachment.

Waldek Hebisch

unread,
Sep 9, 2020, 6:30:51 AM9/9/20
to fricas...@googlegroups.com
Oops, second time.
--
Waldek Hebisch
cp2.diff

oldk1331

unread,
Sep 10, 2020, 9:17:46 AM9/10/20
to fricas...@googlegroups.com
I tested, it works fine, please commit.

--
You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.

Qian Yun

unread,
Jun 13, 2022, 11:04:18 AM6/13/22
to fricas...@googlegroups.com
Hi Waldek,

The original problem still exists:

I can still get
"*** buffer overflow detected ***: terminated"
when PID goes over 1 million.

That's because on my system GCC has auto hardening feature.
(This feature should be popular on other distros as well.)
To be precise, GCC automatically compiles with
"-D_FORTIFY_SOURCE=2". This is adding runtime checks
that gives this particular error.

Turning it down to "-D_FORTIFY_SOURCE=1" can prevent
this problem.

However, I still think using "0-9a-z" to encode socket
address is a better solution.

- Qian

Qian Yun

unread,
Jun 17, 2022, 8:47:12 AM6/17/22
to fricas...@googlegroups.com
And we do not propagate CFLAGS properly, so we can not
use "make CFLAGS=-D_FORTIFY_SOURCE=1" to slip in this
workaround.

- Qian

Qian Yun

unread,
Jun 17, 2022, 8:59:03 AM6/17/22
to fricas...@googlegroups.com
Hello everyone,

I would like to know if your system is affected as well.
You can simply bump your system's next PID by:

echo 1000000 | sudo tee /proc/sys/kernel/ns_last_pid

Run "fricas" and see if it has buffer overflow error message.

If you do have such error message, please reply this email
with your Linux distro's version.

If you do not have problem, you can reply if you like.

- Thanks,
- Qian

Bill Page

unread,
Jun 17, 2022, 12:19:41 PM6/17/22
to fricas...@googlegroups.com
When I do this

wspage@ASUS:~/Desktop$ echo 1000000 | sudo tee /proc/sys/kernel/ns_last_pid

and then run fricas I get

wspage@ASUS:~/Desktop$ fricas

debugger invoked on a SIMPLE-ERROR in thread
#<THREAD "main thread" RUNNING {10005C85B3}>:
Error opening shared object "libssl.so.1.0.0":
libssl.so.1.0.0: cannot open shared object file: No such file or directory.

Type HELP for debugger help, or (SB-EXT:EXIT) to exit from SBCL.

restarts (invokable by number or by possibly-abbreviated name):
0: [CONTINUE ] Skip this shared object and continue.
1: [RETRY ] Retry loading this shared object.
2: [CHANGE-PATHNAME] Specify a different pathname to load the shared
object from.
3: [ABORT ] Exit from the current thread.

(SB-SYS:DLOPEN-OR-LOSE #S(SB-ALIEN::SHARED-OBJECT :PATHNAME
#P"libssl.so.1.0.0" :NAMESTRING "libssl.so.1.0.0" :HANDLE NIL
:DONT-SAVE NIL))
0] (HyperDoc) Error opening FriCAS server. Retrying ...
(HyperDoc) Couldn't connect to FriCAS server!
(HyperDoc) Couldn't connect to FriCAS server!
^C
^C does not break out of the loop and the session appears hung. So ...
^Z
[1]+ Stopped fricas
wspage@ASUS:~/Desktop$ kill %1
> --
> You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/fricas-devel/c7fcd63a-e746-bbf0-3a4a-160efe4dc485%40gmail.com.

Bill Page

unread,
Jun 17, 2022, 12:23:24 PM6/17/22
to fricas...@googlegroups.com
Sorry, I forgot you also wanted to know which Linux.

wspage@ASUS:~/Desktop$ uname -r
5.4.0-120-generic
wspage@ASUS:~/Desktop$ cat /etc/os-release
NAME="Linux Mint"
VERSION="20.3 (Una)"
ID=linuxmint
ID_LIKE=ubuntu
PRETTY_NAME="Linux Mint 20.3"
VERSION_ID="20.3"
HOME_URL="https://www.linuxmint.com/"
SUPPORT_URL="https://forums.linuxmint.com/"
BUG_REPORT_URL="http://linuxmint-troubleshooting-guide.readthedocs.io/en/latest/"
PRIVACY_POLICY_URL="https://www.linuxmint.com/"
VERSION_CODENAME=una
UBUNTU_CODENAME=focal

Kurt Pagani

unread,
Jun 17, 2022, 6:53:41 PM6/17/22
to fricas...@googlegroups.com
On 17.06.2022 14:58, Qian Yun wrote:
> echo 1000000 | sudo tee /proc/sys/kernel/ns_last_pid
Hi Qian
Indeed!

---
fp@sirius:~$ echo 1000000 | sudo tee /proc/sys/kernel/ns_last_pid

1000000

kfp@sirius:~$ fricas

viewman not present, disabling graphics
hypertex not present, disabling
*** buffer overflow detected ***: terminated
Aborted (core dumped)

kfp@sirius:~$ uname -a
Linux sirius 5.13.0-48-generic #54-Ubuntu SMP Wed Jun 1 20:38:48 UTC 2022 x86_64
x86_64 x86_64 GNU/Linux

kfp@sirius:~$ cat /etc/os-release
PRETTY_NAME="Ubuntu 21.10"
NAME="Ubuntu"
VERSION_ID="21.10"
VERSION="21.10 (Impish Indri)"
VERSION_CODENAME=impish
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
UBUNTU_CODENAME=impish

Waldek Hebisch

unread,
Jun 18, 2022, 7:31:59 AM6/18/22
to fricas...@googlegroups.com
On Fri, Jun 17, 2022 at 08:46:23PM +0800, Qian Yun wrote:
> And we do not propagate CFLAGS properly, so we can not
> use "make CFLAGS=-D_FORTIFY_SOURCE=1" to slip in this
> workaround.

If you do

export CC="gcc -D_FORTIFY_SOURCE=1"

before configure and make, than all compilations will use
provided CC, so the flag will be used. Note that
configure and build in principle should use the same
CFLAGS so passing CFLAGS as argument to make is
debatable from correctness point of view.
"Properly passing CFLAGS" can mean many things. One
possible meaning is to ensure that CFLAGS used
during configure are also used during build. Another
could be trying to blend user-provided CFLAGS with
parts that we need. Trying to simply override
CFLAGS in principle can fail, so adding "always
needed" flags to CC is probably simplest way.

--
Waldek Hebisch

Waldek Hebisch

unread,
Jun 18, 2022, 7:44:29 AM6/18/22
to fricas...@googlegroups.com
On Mon, Jun 13, 2022 at 11:03:29PM +0800, Qian Yun wrote:
> Hi Waldek,
>
> The original problem still exists:
>
> I can still get
> "*** buffer overflow detected ***: terminated"
> when PID goes over 1 million.
>
> That's because on my system GCC has auto hardening feature.
> (This feature should be popular on other distros as well.)
> To be precise, GCC automatically compiles with
> "-D_FORTIFY_SOURCE=2". This is adding runtime checks
> that gives this particular error.
>
> Turning it down to "-D_FORTIFY_SOURCE=1" can prevent
> this problem.
>
> However, I still think using "0-9a-z" to encode socket
> address is a better solution.

There should be way to write the code so that it works
with -D_FORTIFY_SOURCE=2 and large PID-s. Unfortunately
on systems that I use I can not set large PID-s.

--
Waldek Hebisch

Qian Yun

unread,
Jun 19, 2022, 1:26:15 AM6/19/22
to fricas...@googlegroups.com
Why can't your system set large PID? Kernel from last
decade should work. Or you can use a shell script to
quickly bump PID over 1 million.

- Qian

Waldek Hebisch

unread,
Jun 19, 2022, 9:44:58 AM6/19/22
to fricas...@googlegroups.com
Maybe it is kernel customization by distribution. As
I wrote, limit on PID-s is configurable at kernel build
time and can be increased or lowered.

Or your "last decade" is really "last year or two"...

--
Waldek Hebisch

Qian Yun

unread,
Jun 19, 2022, 10:43:02 AM6/19/22
to fricas...@googlegroups.com
Still not clear whether your /proc/sys/kernel/pid_max
is less than 1 million, or you couldn't write to
/proc/sys/kernel/ns_last_pid.

I wonder if you'd like to share your distro's
information and kernel version.

- Qian

Waldek Hebisch

unread,
Jun 19, 2022, 11:11:01 AM6/19/22
to fricas...@googlegroups.com
On Sun, Jun 19, 2022 at 10:42:13PM +0800, Qian Yun wrote:
> Still not clear whether your /proc/sys/kernel/pid_max
> is less than 1 million,

cat /proc/sys/kernel/pid_max
32768

> or you couldn't write to
> /proc/sys/kernel/ns_last_pid.

I can, but write fails when value is larger than pid_max.

> I wonder if you'd like to share your distro's
> information and kernel version.

As I wrote many times: Debian 8 (with several private additions).

uname -a
Linux kompter 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2 (2016-04-08) x86_64 GNU/Linux

--
Waldek Hebisch

Qian Yun

unread,
Jun 19, 2022, 11:25:17 AM6/19/22
to fricas...@googlegroups.com
OK, understood.

So does
sudo sysctl -w kernel.pid_max=2000000
work for you?

- Qian

Ralf Hemmecke

unread,
Jun 19, 2022, 3:19:27 PM6/19/22
to fricas...@googlegroups.com
%>echo 1000000 | sudo tee /proc/sys/kernel/ns_last_pid
1000000
%>fricas
*** buffer overflow detected ***: terminated
Aborted

%>cat /etc/os-release
PRETTY_NAME="Ubuntu 22.04 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
VERSION="22.04 LTS (Jammy Jellyfish)"
VERSION_CODENAME=jammy
UBUNTU_CODENAME=jammy

%>uname -a
Linux foo 5.15.0-37-generic #39-Ubuntu SMP Wed Jun 1 19:16:45 UTC 2022
x86_64 x86_64 x86_64 GNU/Linux

James Cloos

unread,
Jun 19, 2022, 3:22:46 PM6/19/22
to fricas...@googlegroups.com
>>>>> "WH" == Waldek Hebisch <de...@fricas.math.uni.wroc.pl> writes:

WH> cat /proc/sys/kernel/pid_max
WH> 32768

which, btw, is the default value.

-JimC
--
James Cloos <cl...@jhcloos.com> OpenPGP: 0x997A9F17ED7DAEA6

Qian Yun

unread,
Jun 21, 2022, 10:16:29 AM6/21/22
to fricas...@googlegroups.com
Can we try to fix this bug with 'sockaddr_un'?

About windows:

Cygwin should work, because it is an POSIX emulation layer.

Mingw64 (aka native WIN32) never worked: sman was never built
as WIN32. So on (native WIN32) windows, the socket related code
is never used, right?

The WinSock code was there when FriCAS was forked
from Axiom. And Axiom doesn't contain this WinSock code.
Not sure why it was there in the first place.

- Qian

On 7/29/20 10:21, Waldek Hebisch wrote:
> On Sun, Jul 26, 2020 at 06:00:17PM +0800, oldk1331 wrote:
>> Hi all,
>>
>> Today suddenly I can't launch FriCAS, it
>> shows "*** buffer overflow detected ***:
>> terminated Aborted".
>>
>> After some debugging, I found that sman
>> calls function "open_server" in
>> sockio-c.c#929:
>>
>> strcpy(server[1].addr.u_addr.sa_data, name);
>>
>> "sa_data" is char[14], and "name" is
>> "/tmp/.i"+getpid(), so on my system PID is
>> over a million, causing buffer overflow
>> in strcpy.
>>
>> I wonder what's the best way to solve this
>> issue, and other C-string buffer overflow
>> in our code base.
>
> I am not sure about best way. Pragmatically, something
> like:
>
> -- a/src/include/com.h
> +++ b/src/include/com.h
> @@ -70,6 +70,7 @@ typedef struct {
> union {
> struct sockaddr u_addr;
> struct sockaddr_in i_addr;
> + char pad[32];
> } addr;
> char *host_name; /* name of foreign host if type == AF_INET */
> } Sock;
>
> should work. On Linux theoreticaly cleaner would be to use
> 'sockaddr_un' instead of 'sockaddr' (and include 'sys/un.h').
> But Windows needs different declarations. Also, in 'sockio-c.c'
> theorticaly it would be safer to write:
>
> (struct sockaddr *)(&server[1].addr)
>
> instead of '&server[1].addr.u_addr'.
>

Waldek Hebisch

unread,
Jun 26, 2022, 10:00:12 AM6/26/22
to fricas...@googlegroups.com
On Tue, Jun 21, 2022 at 10:15:27PM +0800, Qian Yun wrote:
> Can we try to fix this bug with 'sockaddr_un'?

I am not sure what you mean here. We want ability to
handle both Unix domain sockets and Internet sockets.
With proper conditiononals this should be possible.
So we could use 'sockaddr_un', but with conditionals
making sure that it is used when appropriate (and
only in such cases).

> About windows:
>
> Cygwin should work, because it is an POSIX emulation layer.
>
> Mingw64 (aka native WIN32) never worked: sman was never built
> as WIN32. So on (native WIN32) windows, the socket related code
> is never used, right?
>
> The WinSock code was there when FriCAS was forked
> from Axiom. And Axiom doesn't contain this WinSock code.
> Not sure why it was there in the first place.

WinSock code is due to Gabriel Dos Reis (possibly parts
are due to Mike Thomas who had special Windows patches).
The code was in build-improvements branch and probably
never made it to Tim Daly Axiom.

AFAIK this code compiled on Windows and presumably passed all
tests that Gaby did.

Concerning sman: AFAICS the big trouble here are pty-s, they
are not directly available in Windows and the best one can
do is to provide some kind of emulation. Cygwin provides
emulation of pty-s, I do not know how it is done and if
it is feasible to provide such emulation for Mingw.
However, it seem easier (but not easy) to replace all
uses of pty-s by sockets. Of course, in such scenario
Winsock code would be quite important.

Separate issue is Cygwin, I am not sure if it used
Winsock sockets or Cygwin emulation.

--
Waldek Hebisch

Qian Yun

unread,
Jun 26, 2022, 12:15:05 PM6/26/22
to fricas...@googlegroups.com


On 6/26/22 22:00, Waldek Hebisch wrote:
> On Tue, Jun 21, 2022 at 10:15:27PM +0800, Qian Yun wrote:
>> Can we try to fix this bug with 'sockaddr_un'?
>
> I am not sure what you mean here. We want ability to
> handle both Unix domain sockets and Internet sockets.
> With proper conditiononals this should be possible.
> So we could use 'sockaddr_un', but with conditionals
> making sure that it is used when appropriate (and
> only in such cases).

What I meant is this patch (or see end of email):

https://github.com/oldk1331/fricas/commit/39037b7cb5dff9cff8a42068dbc51fe4e91e33c2.patch

Tested on Linux, fixes the original problem.

>> About windows:
>>
>> Cygwin should work, because it is an POSIX emulation layer.
>>
>> Mingw64 (aka native WIN32) never worked: sman was never built
>> as WIN32. So on (native WIN32) windows, the socket related code
>> is never used, right?
>>
>> The WinSock code was there when FriCAS was forked
>> from Axiom. And Axiom doesn't contain this WinSock code.
>> Not sure why it was there in the first place.
>
> WinSock code is due to Gabriel Dos Reis (possibly parts
> are due to Mike Thomas who had special Windows patches).
> The code was in build-improvements branch and probably
> never made it to Tim Daly Axiom.
>
> AFAIK this code compiled on Windows and presumably passed all
> tests that Gaby did.
>
> Concerning sman: AFAICS the big trouble here are pty-s, they
> are not directly available in Windows and the best one can
> do is to provide some kind of emulation. Cygwin provides
> emulation of pty-s, I do not know how it is done and if
> it is feasible to provide such emulation for Mingw.
> However, it seem easier (but not easy) to replace all
> uses of pty-s by sockets. Of course, in such scenario
> Winsock code would be quite important.
>
> Separate issue is Cygwin, I am not sure if it used
> Winsock sockets or Cygwin emulation.
>

This patch fails on Mingw64.

I believe Cygwin uses emulation of POSIX AF_UNIX. I haven't
try that, but I can test it.

So, for Mingw64, the Winsock code was there, but never used.
Can I disable socket related functions on Mingw64?

BTW, open axiom now uses named pipe on Windows:
https://github.com/GabrielDosReis/open-axiom/blob/master/src/lib/sockio-c.cxx#L252

- Qian

=================

From 39037b7cb5dff9cff8a42068dbc51fe4e91e33c2 Mon Sep 17 00:00:00 2001
From: Qian Yun <oldk...@gmail.com>
Date: Sun, 26 Jun 2022 23:56:38 +0800
Subject: [PATCH] try to use sockaddr_un

---
src/include/com.h | 4 ++--
src/lib/sockio-c.c | 20 ++++++++------------
2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/src/include/com.h b/src/include/com.h
index 4faeb2c1c..465e3a491 100644
--- a/src/include/com.h
+++ b/src/include/com.h
@@ -39,6 +39,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
#else
# include <sys/types.h>
# include <sys/socket.h>
+# include <sys/un.h>
# include <netinet/in.h>
#endif

@@ -68,9 +69,8 @@ typedef struct {
int frame; /* spad interpreter frame (for interpreter
windows) */
fricas_socket remote; /* file descriptor of remote socket */
union {
- struct sockaddr u_addr;
+ struct sockaddr_un u_addr;
struct sockaddr_in i_addr;
- char pad[32];
} addr;
char *host_name; /* name of foreign host if type == AF_INET */
} Sock;
diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
index 5878cfb11..2f6a5edb8 100644
--- a/src/lib/sockio-c.c
+++ b/src/lib/sockio-c.c
@@ -752,13 +752,11 @@ connect_to_local_server(char *server_name, int
purpose, int time_out)
return NULL;
}
/* connect socket using name specified in command line */
- sock->addr.u_addr.sa_family = FRICAS_AF_LOCAL;
- strcpy(sock->addr.pad +
- ((char *)(&(sock->addr.u_addr.sa_data))
- -(char *)(&(sock->addr.u_addr))), name);
+ struct sockaddr_un *uaddr = &(sock->addr.u_addr);
+ uaddr->sun_family = FRICAS_AF_LOCAL;
+ strncpy(uaddr->sun_path, name, sizeof(uaddr->sun_path) - 1);
for(i=0; i<max_con; i++) {
- code = connect(sock->socket, &sock->addr.u_addr,
- sizeof(sock->addr.pad));
+ code = connect(sock->socket, uaddr, sizeof(*uaddr));
if (code == -1) {
if (
/* @@@ Why we need this */
@@ -924,12 +922,10 @@ open_server(char *server_name)
return -2;
} else {
Sock * sock = &(server[1]);
- sock->addr.u_addr.sa_family = FRICAS_AF_LOCAL;
- strcpy(sock->addr.pad +
- ((char *)(&(sock->addr.u_addr.sa_data))
- -(char *)(&(sock->addr.u_addr))), name);
- if (bind(sock->socket, &(sock->addr.u_addr),
- sizeof(sock->addr.pad))) {
+ struct sockaddr_un * uaddr = &(sock->addr.u_addr);
+ uaddr->sun_family = FRICAS_AF_LOCAL;
+ strncpy(uaddr->sun_path, name, sizeof(uaddr->sun_path) - 1);
+ if (bind(sock->socket, uaddr, sizeof(*uaddr))) {
perror("binding local server socket");
server[1].socket = 0;
return -2;

Waldek Hebisch

unread,
Jun 26, 2022, 1:28:06 PM6/26/22
to fricas...@googlegroups.com
On Mon, Jun 27, 2022 at 12:13:58AM +0800, Qian Yun wrote:
>
>
> On 6/26/22 22:00, Waldek Hebisch wrote:
> >On Tue, Jun 21, 2022 at 10:15:27PM +0800, Qian Yun wrote:
> >>Can we try to fix this bug with 'sockaddr_un'?
> >
> >I am not sure what you mean here. We want ability to
> >handle both Unix domain sockets and Internet sockets.
> >With proper conditiononals this should be possible.
> >So we could use 'sockaddr_un', but with conditionals
> >making sure that it is used when appropriate (and
> >only in such cases).
>
> What I meant is this patch (or see end of email):
>
> https://github.com/oldk1331/fricas/commit/39037b7cb5dff9cff8a42068dbc51fe4e91e33c2.patch
>
> Tested on Linux, fixes the original problem.

I was thinking more in direction of having something like:

union {
#if defined(HAVE_SOCK_UN)
struct sockaddr_un u_addr;
#endif
struct sockaddr_in i_addr;

and similar in other places using sockaddr_un. And of course
we would need test in 'configure.ac'.
For now yes. But we would like to be able to use some
alternative, so local conditionals like I suggest seem
better than say skipping entire file.
IIUC firewalls on Windows got too eager and stop local connections.
Named pipes give similar effect as Unix domain socket, in particular
avoid exposure to Internet, so such change makes sense.

--
Waldek Hebisch

Qian Yun

unread,
Jun 27, 2022, 6:10:36 AM6/27/22
to fricas...@googlegroups.com


On 6/27/22 01:28, Waldek Hebisch wrote:
>
> I was thinking more in direction of having something like:
>
> union {
> #if defined(HAVE_SOCK_UN)
> struct sockaddr_un u_addr;
> #endif
> struct sockaddr_in i_addr;
>
> and similar in other places using sockaddr_un. And of course
> we would need test in 'configure.ac'.
>

See branch at https://github.com/oldk1331/fricas/commits/fix-socket-unix
Or the following diff.

There can be further refinement, such as adjust "fricas_host_has_socket"
in configure.ac.

- Qian

===================

diff --git a/config/fricas_c_macros.h.in b/config/fricas_c_macros.h.in
index 6fbbd172..04ec5cf8 100644
--- a/config/fricas_c_macros.h.in
+++ b/config/fricas_c_macros.h.in
@@ -115,6 +115,9 @@
/* Define to 1 if you have the <sys/types.h> header file. */
#undef HAVE_SYS_TYPES_H

+/* Define to 1 if you have the <sys/un.h> header file. */
+#undef HAVE_SYS_UN_H
+
/* Define to 1 if you have the <sys/wait.h> header file. */
#undef HAVE_SYS_WAIT_H

diff --git a/configure b/configure
index 16d4948f..3704f569 100755
--- a/configure
+++ b/configure
@@ -5089,12 +5089,13 @@ done
fricas_c_runtime_extra="-lwsock32"
;;
*)
- for ac_header in sys/socket.h
+ for ac_header in sys/socket.h sys/un.h
do :
- ac_fn_c_check_header_mongrel "$LINENO" "sys/socket.h"
"ac_cv_header_sys_socket_h" "$ac_includes_default"
-if test "x$ac_cv_header_sys_socket_h" = xyes; then :
+ as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
+ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header"
"$ac_includes_default"
+if eval test \"x\$"$as_ac_Header"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_SYS_SOCKET_H 1
+#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1
_ACEOF
fricas_host_has_socket=yes
fi
diff --git a/configure.ac b/configure.ac
index 46225a66..bae4b673 100644
--- a/configure.ac
+++ b/configure.ac
@@ -555,7 +555,7 @@ case $host in
fricas_c_runtime_extra="-lwsock32"
;;
*)
- AC_CHECK_HEADERS([sys/socket.h],
+ AC_CHECK_HEADERS([sys/socket.h sys/un.h],
[fricas_host_has_socket=yes],
[])
;;
diff --git a/src/include/com.h b/src/include/com.h
index 465e3a49..0f8f9911 100644
--- a/src/include/com.h
+++ b/src/include/com.h
@@ -69,7 +69,9 @@ typedef struct {
int frame; /* spad interpreter frame (for interpreter
windows) */
fricas_socket remote; /* file descriptor of remote socket */
union {
+ #ifdef HAVE_SYS_UN_H
struct sockaddr_un u_addr;
+ #endif
struct sockaddr_in i_addr;
} addr;
char *host_name; /* name of foreign host if type == AF_INET */
diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
index 2f6a5edb..47f809a0 100644
--- a/src/lib/sockio-c.c
+++ b/src/lib/sockio-c.c
@@ -751,6 +751,9 @@ connect_to_local_server(char *server_name, int
purpose, int time_out)
free(sock);
return NULL;
}
+#ifndef HAVE_SYS_UN_H
+ return NULL;
+#else
/* connect socket using name specified in command line */
struct sockaddr_un *uaddr = &(sock->addr.u_addr);
uaddr->sun_family = FRICAS_AF_LOCAL;
@@ -783,6 +786,7 @@ connect_to_local_server(char *server_name, int
purpose, int time_out)
/* fprintf(stderr, "Got int form socket\n"); */
sock->remote = get_int(sock);
return sock;
+#endif
}

/* act as terminal session for sock connected to stdin and stdout of
another
@@ -915,6 +919,10 @@ open_server(char *server_name)
listen(server[0].socket,5);
} */
/* Next create the local domain socket */
+#ifndef HAVE_SYS_UN_H
+ server[1].socket = 0;
+ return -1;
+#else
server[1].socket = fricas_communication_link(FRICAS_AF_LOCAL);
if (is_invalid_socket(&server[1])) {
perror("opening local server socket");
@@ -946,6 +954,7 @@ open_server(char *server_name)
return -1;
}
return 0;
+#endif
}

int

Waldek Hebisch

unread,
Jun 27, 2022, 7:14:04 PM6/27/22
to fricas...@googlegroups.com
On Mon, Jun 27, 2022 at 06:09:28PM +0800, Qian Yun wrote:
>
> See branch at https://github.com/oldk1331/fricas/commits/fix-socket-unix
> Or the following diff.
>
> There can be further refinement, such as adjust "fricas_host_has_socket"
> in configure.ac.

That looks mostly OK. The little possible problem is choosing
good test in configure. We probably need some testing to find
out if the test for header is good one.

--
Waldek Hebisch

Qian Yun

unread,
Jun 28, 2022, 5:52:02 AM6/28/22
to fricas...@googlegroups.com
Updated, see:
https://github.com/oldk1331/fricas/commit/6af9de34eaab919ebb650e2bf00ac012f61a4aeb.patch

Or see at the end of this mail.

- Qian
======

From 6af9de34eaab919ebb650e2bf00ac012f61a4aeb Mon Sep 17 00:00:00 2001
From: Qian Yun <oldk...@gmail.com>
Date: Sun, 26 Jun 2022 23:56:38 +0800
Subject: [PATCH] Use sockaddr_un on POSIX systems

---
config/fricas_c_macros.h.in | 6 ++++++
configure | 32 ++++++++++++++++++++++++++++----
configure.ac | 10 +++++++++-
src/include/com.h | 6 ++++--
src/lib/sockio-c.c | 29 +++++++++++++++++------------
5 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/config/fricas_c_macros.h.in b/config/fricas_c_macros.h.in
index 6fbbd1728..edc984226 100644
--- a/config/fricas_c_macros.h.in
+++ b/config/fricas_c_macros.h.in
@@ -91,6 +91,9 @@
/* Define to 1 if you have the <signal.h> header file. */
#undef HAVE_SIGNAL_H

+/* Host has sockaddr_un */
+#undef HAVE_SOCKADDR_UN
+
/* Host has SO_NOSIGPIPE */
#undef HAVE_SO_NOSIGPIPE

@@ -115,6 +118,9 @@
/* Define to 1 if you have the <sys/types.h> header file. */
#undef HAVE_SYS_TYPES_H

+/* Define to 1 if you have the <sys/un.h> header file. */
+#undef HAVE_SYS_UN_H
+
/* Define to 1 if you have the <sys/wait.h> header file. */
#undef HAVE_SYS_WAIT_H

diff --git a/configure b/configure
index 16d4948fa..c80def0b8 100755
--- a/configure
+++ b/configure
@@ -5089,12 +5089,13 @@ done
fricas_c_runtime_extra="-lwsock32"
;;
*)
- for ac_header in sys/socket.h
+ for ac_header in sys/socket.h sys/un.h
do :
- ac_fn_c_check_header_mongrel "$LINENO" "sys/socket.h"
"ac_cv_header_sys_socket_h" "$ac_includes_default"
-if test "x$ac_cv_header_sys_socket_h" = xyes; then :
+ as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
+ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header"
"$ac_includes_default"
+if eval test \"x\$"$as_ac_Header"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_SYS_SOCKET_H 1
+#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1
_ACEOF
fricas_host_has_socket=yes
fi
@@ -5175,6 +5176,29 @@ rm -f core conftest.err conftest.$ac_objext
conftest.$ac_ext
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */

+#include <sys/socket.h>
+#include <sys/un.h>
+
+int
+main ()
+{
+
+struct sockaddr_un uaddr;
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+$as_echo "#define HAVE_SOCKADDR_UN 1" >>confdefs.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
#include <sys/stat.h>
#include <sys/types.h>

diff --git a/configure.ac b/configure.ac
index 46225a668..cfc74ef1c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -555,7 +555,7 @@ case $host in
fricas_c_runtime_extra="-lwsock32"
;;
*)
- AC_CHECK_HEADERS([sys/socket.h],
+ AC_CHECK_HEADERS([sys/socket.h sys/un.h],
[fricas_host_has_socket=yes],
[])
;;
@@ -584,6 +584,14 @@ int code = EPIPE;
],
[AC_DEFINE([HAVE_EPIPE], [1], [Host has EPIPE])],[])

+AC_TRY_COMPILE([
+#include <sys/socket.h>
+#include <sys/un.h>
+ ], [
+struct sockaddr_un uaddr;
+ ],
+ [AC_DEFINE([HAVE_SOCKADDR_UN], [1], [Host has sockaddr_un])],[])
+
AC_TRY_COMPILE([
#include <sys/stat.h>
#include <sys/types.h>
diff --git a/src/include/com.h b/src/include/com.h
index 4faeb2c1c..470018b9c 100644
--- a/src/include/com.h
+++ b/src/include/com.h
@@ -39,6 +39,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
DAMAGE.
#else
# include <sys/types.h>
# include <sys/socket.h>
+# include <sys/un.h>
# include <netinet/in.h>
#endif

@@ -68,9 +69,10 @@ typedef struct {
int frame; /* spad interpreter frame (for interpreter
windows) */
fricas_socket remote; /* file descriptor of remote socket */
union {
- struct sockaddr u_addr;
+ #ifdef HAVE_SOCKADDR_UN
+ struct sockaddr_un u_addr;
+ #endif
struct sockaddr_in i_addr;
- char pad[32];
} addr;
char *host_name; /* name of foreign host if type == AF_INET */
} Sock;
diff --git a/src/lib/sockio-c.c b/src/lib/sockio-c.c
index 5878cfb11..fb9e70593 100644
--- a/src/lib/sockio-c.c
+++ b/src/lib/sockio-c.c
@@ -751,14 +751,15 @@ connect_to_local_server(char *server_name, int
purpose, int time_out)
free(sock);
return NULL;
}
+#ifndef HAVE_SOCKADDR_UN
+ return NULL;
+#else
/* connect socket using name specified in command line */
- sock->addr.u_addr.sa_family = FRICAS_AF_LOCAL;
- strcpy(sock->addr.pad +
- ((char *)(&(sock->addr.u_addr.sa_data))
- -(char *)(&(sock->addr.u_addr))), name);
+ struct sockaddr_un *uaddr = &(sock->addr.u_addr);
+ uaddr->sun_family = FRICAS_AF_LOCAL;
+ strncpy(uaddr->sun_path, name, sizeof(uaddr->sun_path) - 1);
for(i=0; i<max_con; i++) {
- code = connect(sock->socket, &sock->addr.u_addr,
- sizeof(sock->addr.pad));
+ code = connect(sock->socket, uaddr, sizeof(*uaddr));
if (code == -1) {
if (
/* @@@ Why we need this */
@@ -785,6 +786,7 @@ connect_to_local_server(char *server_name, int
purpose, int time_out)
/* fprintf(stderr, "Got int form socket\n"); */
sock->remote = get_int(sock);
return sock;
+#endif
}

/* act as terminal session for sock connected to stdin and stdout of
another
@@ -917,6 +919,10 @@ open_server(char *server_name)
listen(server[0].socket,5);
} */
/* Next create the local domain socket */
+#ifndef HAVE_SOCKADDR_UN
+ server[1].socket = 0;
+ return -1;
+#else
server[1].socket = fricas_communication_link(FRICAS_AF_LOCAL);
if (is_invalid_socket(&server[1])) {
perror("opening local server socket");
@@ -924,12 +930,10 @@ open_server(char *server_name)
return -2;
} else {
Sock * sock = &(server[1]);
- sock->addr.u_addr.sa_family = FRICAS_AF_LOCAL;
- strcpy(sock->addr.pad +
- ((char *)(&(sock->addr.u_addr.sa_data))
- -(char *)(&(sock->addr.u_addr))), name);
- if (bind(sock->socket, &(sock->addr.u_addr),
- sizeof(sock->addr.pad))) {
+ struct sockaddr_un * uaddr = &(sock->addr.u_addr);
+ uaddr->sun_family = FRICAS_AF_LOCAL;
+ strncpy(uaddr->sun_path, name, sizeof(uaddr->sun_path) - 1);
+ if (bind(sock->socket, uaddr, sizeof(*uaddr))) {
perror("binding local server socket");
server[1].socket = 0;
return -2;
@@ -950,6 +954,7 @@ open_server(char *server_name)
Reply all
Reply to author
Forward
0 new messages