[PATCH] load lib/libspad.so correctly when "FRICAS" env var is not set

[Qian Yun]

When launching "FRICASsys" from terminal without environment variable
"FRICAS" set, you get following message:

Checking for foreign routines
FRICAS=NIL
spad-lib="/lib/libspad.so"

Then it will look for "/lib/libspad.so" to load and will fail.

It will affect functionalities like ")compile", because it will
need the "remove_directory" C function.

This can be a common scenario: during debugging, you may have
multiple directories containing different "FRICASsys", it can
be convenient to run the binary directly without setting different
"FRICAS" first. Also this is the current status of our Windows
distribution: we only have "bin/FRICASsys.exe" and "FRICAS" is not
set in Windows environment variables.

So the following patch is to deal with this case -- when "FRICAS"
is not set. There is already logic in function "initroot" to set
variable "$spadroot" by probing parent directory.

- Qian

====

diff --git a/src/interp/util.lisp b/src/interp/util.lisp
index 63cd20f3..2a6219cd 100644
--- a/src/interp/util.lisp
+++ b/src/interp/util.lisp
@@ -77,7 +77,7 @@
;;; It is set up in the {\bf reroot} function.
(defvar $library-directory-list ())

-;;; Prefix a filename with the {\bf FRICAS} shell variable.
+;;; Prefix a filename with the {\bf $spadroot} variable.
(defun make-absolute-filename (name)
(concatenate 'string $spadroot name))

@@ -88,13 +88,10 @@
be {\bf \$spadroot}. The {\bf reroot} function will change the
system to use a new root directory and will have the same effect
as changing the {\bf FRICAS} shell variable and rerunning the system
-from scratch. A correct call looks like:
-\begin{verbatim}
-(in-package "BOOT")
-(reroot "${FRICAS}")
-\end{verbatim}
-where the [[${FRICAS}]] variable points to installed tree.
+from scratch.
|#
+(defvar $spadroot "")
+
(defun reroot (dir)
(setq $spadroot dir)
(setq $directory-list
@@ -267,10 +264,9 @@
#+:GCL (system:gbc-time 0)
#+(or :sbcl :clisp :openmcl :lispworks)
(if *fricas-load-libspad*
- (let* ((ax-dir (|getEnv| "FRICAS"))
- (spad-lib (concatenate 'string ax-dir "/lib/libspad.so")))
+ (let ((spad-lib (make-absolute-filename "/lib/libspad.so")))
(format t "Checking for foreign routines~%")
- (format t "FRICAS=~S~%" ax-dir)
+ (format t "FRICAS=~S~%" $spadroot)
(format t "spad-lib=~S~%" spad-lib)
(if (|fricas_probe_file| spad-lib)
(progn
@@ -280,7 +276,7 @@
(|quiet_load_alien| spad-lib)
#+(or :sbcl :openmcl)
(fricas-lisp::init-gmp
- (concatenate 'string ax-dir "/lib/gmp_wrap.so"))
+ (make-absolute-filename "/lib/gmp_wrap.so"))
#+(and :clisp :ffi)
(progn
(eval `(FFI:DEFAULT-FOREIGN-LIBRARY ,spad-lib))

[Waldek Hebisch]

Dnia Wed, Sep 20, 2023 at 06:20:16PM +0800, Qian Yun napisał(a):
> When launching "FRICASsys" from terminal without environment variable
> "FRICAS" set, you get following message:
>
> Checking for foreign routines
> FRICAS=NIL
> spad-lib="/lib/libspad.so"
>
> Then it will look for "/lib/libspad.so" to load and will fail.
>
> It will affect functionalities like ")compile", because it will
> need the "remove_directory" C function.
>
> This can be a common scenario: during debugging, you may have
> multiple directories containing different "FRICASsys", it can
> be convenient to run the binary directly without setting different
> "FRICAS" first. Also this is the current status of our Windows
> distribution: we only have "bin/FRICASsys.exe" and "FRICAS" is not
> set in Windows environment variables.
>
> So the following patch is to deal with this case -- when "FRICAS"
> is not set. There is already logic in function "initroot" to set
> variable "$spadroot" by probing parent directory.

The code look fine. But I have mixed feeling about motivation.
Namely, if FRICAS is different than '$spadroot', then we are
going to have troubles sooner or later. On Linux when
debugging freshly build FriCAS I use command line like:

(export FRICAS=/sklad/fricas/axp1/fr-build92/target/x86_64-linux-gnu ; $FRICAS/bin/fricas)

This is long, but is build using bash filename completition, so
is moderate amount of typing. And once build the command sits
in command history and is easy to recall.

Somewhat related to this: I am not sure if we want to support
running without 'libspad.so'. This was allowed because early
Sage packaged FriCAS used clisp without support for foreign
interface. It is not clear to me if there is real need now to
run without 'libspad.so'.

> --
> 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/96fede8a-cba3-4c4a-9a8f-665bfb93d656%40gmail.com.

--
Waldek Hebisch

[Qian Yun]

On 9/21/23 05:44, Waldek Hebisch wrote:
>
> The code look fine. But I have mixed feeling about motivation.
> Namely, if FRICAS is different than '$spadroot', then we are
> going to have troubles sooner or later.

"$spadroot" is initialized to "FRICAS" during startup. This patch
basically deals with the corner case when "FRICAS" is not set.

I'll commit this patch later.

- Qian

[Grégory Vanuxem]

Hi here,

I am in the process of adding this patch to my personal copy of FriCAS
and apparently some strange things happen.So I build vanilla FriCAS
just Git cloned with the today two patches of Qian. Apparently
$spadroot is hardcoded during the build (or something else):

_From my $HOME directory_ without the $FRICAS variable set:

┌──(greg㉿ellipse)-[~]
└─$ /usr/local/lib/fricas/target/x86_64-linux-gnu/bin/FRICASsys
Checking for foreign routines
FRICAS="/home/greg/Tmp/fricas/target/x86_64-linux-gnu"
spad-lib="/home/greg/Tmp/fricas/target/x86_64-linux-gnu/lib/libspad.so"
foreign routines found
openServer result -2
[...]

I wonder if this is the expected behaviour.

Regards,
__
Greg

> --
> 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/0d8f7aee-b610-4b26-877c-5198a4cc14c7%40gmail.com.

[Grégory Vanuxem]

UPDATE

if I delete the build directory, home/greg//Tmp/fricas, all is right.

[Waldek Hebisch]

On Sat, Sep 23, 2023 at 04:27:36PM +0200, Grégory Vanuxem wrote:
> UPDATE
>
> if I delete the build directory, home/greg//Tmp/fricas, all is right.
>

<snip>

> > └─$ /usr/local/lib/fricas/target/x86_64-linux-gnu/bin/FRICASsys
> > Checking for foreign routines
> > FRICAS="/home/greg/Tmp/fricas/target/x86_64-linux-gnu"
> > spad-lib="/home/greg/Tmp/fricas/target/x86_64-linux-gnu/lib/libspad.so"

Unfortunately, this is expected effect of our current logic.
We probably should reset $spadroot to NIL when dumping an image.
Or maybe we should do this at image startup.

Security folks consider hardcoded paths to build directories
as voulnerability: when FriCAS is installed systemwide person
who can write to hardcoded location can cause other folks
to execute arbitrary program instead of FriCAS.

--
Waldek Hebisch

[Grégory Vanuxem]

Le sam. 23 sept. 2023, 17:16, Waldek Hebisch <de...@fricas.org> a écrit :

On Sat, Sep 23, 2023 at 04:27:36PM +0200, Grégory Vanuxem wrote:
> UPDATE
>
> if I delete the build directory, home/greg//Tmp/fricas, all is right.
>
<snip>
> > └─$ /usr/local/lib/fricas/target/x86_64-linux-gnu/bin/FRICASsys
> > Checking for foreign routines
> > FRICAS="/home/greg/Tmp/fricas/target/x86_64-linux-gnu"
> > spad-lib="/home/greg/Tmp/fricas/target/x86_64-linux-gnu/lib/libspad.so"

Unfortunately, this is expected effect of our current logic.
We probably should reset $spadroot to NIL when dumping an image.
Or maybe we should do this at image startup.

Personally I would reset it but I do not know the startup logic as of now, I guess this is necessary somewhere. 

Security folks consider hardcoded paths to build directories
as voulnerability: when FriCAS is installed systemwide person
who can write to hardcoded location can cause other folks
to execute arbitrary program instead of FriCAS.

This is approximately what I thought. More precisely one can think about a completely broken libspad.so in the hardcoded path because of work on it, the system wide FriCAS will also be broken. 

In any case many thanks for this information, so I will push the util.lisp modifications, I wanted to know that. 

__

Greg

--
                              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/ZQ8BSJ4c%2B3q56aJh%40fricas.org.

[Qian Yun]

On one hand, this is the behavior described in comment:
$spadroot was $FRICAS during build time.

;;; Sets up the system to use the {\bf FRICAS} shell variable if we can
;;; and default to the {\bf \$spadroot} variable (which was the value
;;; of the {\bf FRICAS} shell variable at build time) if we can't.
;;; Use the parent directory of FRICASsys binary as fallback.
(defun initroot (&optional (newroot nil))

On the other hand, this mechanism seems not used during build:
It is called in "interpsys-image-init" which involves INTERPSYS,
but all INTERPSYS calls have valid $FRICAS to it.

====

Following patch sets $spadroot to empty string before dump lisp image.
This builds fine and tests are passed.

A complete patch will need to correct previously mentioned comment
and need more thorough review.

- Qian

diff --git a/src/interp/util.lisp b/src/interp/util.lisp
index ffce3d1f..3dab3e77 100644
--- a/src/interp/util.lisp
+++ b/src/interp/util.lisp
@@ -330,6 +330,7 @@
(defun spad-save (save-file do-restart)
(setq |$SpadServer| nil)
(setq $openServerIfTrue t)
+ (setq $spadroot "")
(FRICAS-LISP::save-core-restart save-file
(if do-restart #'boot::fricas-restart nil))
)

[Grégory Vanuxem]

Qian,

Le dim. 24 sept. 2023 à 02:20, Qian Yun <oldk...@gmail.com> a écrit :


[snip]

> diff --git a/src/interp/util.lisp b/src/interp/util.lisp
> index ffce3d1f..3dab3e77 100644
> --- a/src/interp/util.lisp
> +++ b/src/interp/util.lisp
> @@ -330,6 +330,7 @@
> (defun spad-save (save-file do-restart)
> (setq |$SpadServer| nil)
> (setq $openServerIfTrue t)
> + (setq $spadroot "")
> (FRICAS-LISP::save-core-restart save-file
> (if do-restart #'boot::fricas-restart nil))
> )

NICE!

Of course more tests are eventually needed but with this patch that's
even better than when I removed /home/greg/Tmp/fricas.
See my previous mail.
When I removed this directory, at startup, the banner displayed paths
with something like "../../ etc"

Now:
Checking for foreign routines
FRICAS="/usr/local/lib/fricas/target/x86_64-linux-gnu"
spad-lib="/usr/local/lib/fricas/target/x86_64-linux-gnu/lib/libspad.so"

foreign routines found
openServer result -2

FriCAS Computer Algebra System
Version: FriCAS 2023-06-17
Timestamp: Sun Sep 24 04:20:25 AM CEST 2023
-----------------------------------------------------------------------------
Issue )copyright to view copyright notices.
Issue )summary for a summary of useful system commands.
Issue )quit to leave FriCAS and return to shell.
-----------------------------------------------------------------------------

(1) -> )version
"FriCAS 2023-06-17 compiled at Sun Sep 24 04:20:25 AM CEST 2023"
(1) -> )lisp (lisp-implementation-type)

Value = "sbcl"

'lsof' shows that "gmp_wrap.so" is the good one.

Thanks again ;)
__
Greg

>
> On 9/23/23 23:16, Waldek Hebisch wrote:
> > On Sat, Sep 23, 2023 at 04:27:36PM +0200, Grégory Vanuxem wrote:
> >> UPDATE
> >>
> >> if I delete the build directory, home/greg//Tmp/fricas, all is right.
> >>
> > <snip>
> >>> └─$ /usr/local/lib/fricas/target/x86_64-linux-gnu/bin/FRICASsys
> >>> Checking for foreign routines
> >>> FRICAS="/home/greg/Tmp/fricas/target/x86_64-linux-gnu"
> >>> spad-lib="/home/greg/Tmp/fricas/target/x86_64-linux-gnu/lib/libspad.so"
> >
> > Unfortunately, this is expected effect of our current logic.
> > We probably should reset $spadroot to NIL when dumping an image.
> > Or maybe we should do this at image startup.
> >
> > Security folks consider hardcoded paths to build directories
> > as voulnerability: when FriCAS is installed systemwide person
> > who can write to hardcoded location can cause other folks
> > to execute arbitrary program instead of FriCAS.
> >
>

> --
> 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/82eb62bf-4503-40b2-8137-425c44949791%40gmail.com.

[Waldek Hebisch]

On Sun, Sep 24, 2023 at 08:20:45AM +0800, Qian Yun wrote:
> On one hand, this is the behavior described in comment:
> $spadroot was $FRICAS during build time.
>
> ;;; Sets up the system to use the {\bf FRICAS} shell variable if we can
> ;;; and default to the {\bf \$spadroot} variable (which was the value
> ;;; of the {\bf FRICAS} shell variable at build time) if we can't.
> ;;; Use the parent directory of FRICASsys binary as fallback.
> (defun initroot (&optional (newroot nil))
>
> On the other hand, this mechanism seems not used during build:
> It is called in "interpsys-image-init" which involves INTERPSYS,
> but all INTERPSYS calls have valid $FRICAS to it.

This behaviour clearly was intended as a convenience feature for
developers. But times have changed and FriCAS code changed,
so it is now not needed.

ATM it is not clear to me what should we write in the comment.
Most of the comment is redundant, it repeats in impreciase
way what is clearly expressed by code. And extra info
(about $spadroot) is going to be invalid.

To say the truth, it is not clear to me if initroot should
look at $spadroot at all.

Currently we have 4 ways to get base location of FriCAS:

1) FRICAS environment variable
2) argument to build-interpsys
3) location inferred from path to FriCASsys
4) remembered value of $spadroot

ATM I do not know if remembered $spadroot can be used except for
values restored from an image (which is currently used but should
be not used).

> --
> 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/82eb62bf-4503-40b2-8137-425c44949791%40gmail.com.

--
Waldek Hebisch

[Waldek Hebisch]

Removing use of $spadroot from initroot gives sucessful build and
testsuite passes. Starting FRICASsys without seting FRICAS
gives

FRICAS="target/x86_64-linux-gnu/bin//../"
spad-lib="target/x86_64-linux-gnu/bin//..//lib/libspad.so"

which works, but since paths are relative could be broken by
change of current directory:

(1) -> )cd ..
The current FriCAS default directory is /home/hebisch/fricas/axp1
(1) -> integrate(sin(x), x)

>> System error:
Couldn't load "target/x86_64-linux-gnu/bin//..//algebra/EXPR.fasl": file does
not exist.

Also, double slashes may cause trouble (IIRC GCL had trouble with
double slashes.

--
Waldek Hebisch

[Qian Yun]

On 9/24/23 21:42, Waldek Hebisch wrote:
>
> This behaviour clearly was intended as a convenience feature for
> developers. But times have changed and FriCAS code changed,
> so it is now not needed.
>
> ATM it is not clear to me what should we write in the comment.
> Most of the comment is redundant, it repeats in impreciase
> way what is clearly expressed by code. And extra info
> (about $spadroot) is going to be invalid.
>
> To say the truth, it is not clear to me if initroot should
> look at $spadroot at all.
>
> Currently we have 4 ways to get base location of FriCAS:
>
> 1) FRICAS environment variable
> 2) argument to build-interpsys
> 3) location inferred from path to FriCASsys
> 4) remembered value of $spadroot
>
> ATM I do not know if remembered $spadroot can be used except for
> values restored from an image (which is currently used but should
> be not used).
>

The logic should simplifies to:

1) use $FRICAS env var, and store it to $spadroot
2) infer from path to FRICASsys, and store it to $spadroot

Then everywhere else should use $spadroot.

In this case, code in various places can be simplified.

- Qian

[Qian Yun]

On 9/24/23 22:41, Waldek Hebisch wrote:
>
> Removing use of $spadroot from initroot gives sucessful build and
> testsuite passes. Starting FRICASsys without seting FRICAS
> gives
>
> FRICAS="target/x86_64-linux-gnu/bin//../"
> spad-lib="target/x86_64-linux-gnu/bin//..//lib/libspad.so"
>
> which works, but since paths are relative could be broken by
> change of current directory:
>
> (1) -> )cd ..
> The current FriCAS default directory is /home/hebisch/fricas/axp1
> (1) -> integrate(sin(x), x)
>
> >> System error:
> Couldn't load "target/x86_64-linux-gnu/bin//..//algebra/EXPR.fasl": file does
> not exist.
>
> Also, double slashes may cause trouble (IIRC GCL had trouble with
> double slashes.
>

We can get rid of relative path and double slashes by lisp function
"truename".

I'll make this change later if there are no objections.

- Qian