Re: org-persist files in /tmp

2022-12-25 Thread tomas
On Sun, Dec 25, 2022 at 09:30:00AM +, Ihor Radchenko wrote:
>  writes:
> 
> >> Another idea is to avoid caching of parse result for small files.
> >
> > I haven't been following along very closely, but seeing the
> > involved complexities, I'd appreciate a knob to disable caching
> > altogether.
> 
> org-element-cache-persistent

Thanks, Ihor. You made me a Christmas present *<:-)~

> > My usage of Org hasn't triggered any slowdowns which would be
> > worth all that complexity (and yes, my biggest Org file so-far
> > is 36k lines, and my box isn't the fastest around).
> 
> The main reason we cannot disable org-persist is _not_ Org AST cache.
> The reason is mostly caching some downloaded files.

I see. That latency weighs significantly more, yes.

Cheers
-- 
t


signature.asc
Description: PGP signature


Re: org-persist files in /tmp

2022-12-25 Thread Ihor Radchenko
Max Nikulin  writes:

> If you demonstrate that e.g., when working with encrypted files, their 
> sensitive content leaks to the cache then it will raise the severity of 
> the issue.

Nothing related to encrypted files is ever written by org-persist.
Some edge cases might exist for org-crypt, but that will involve manual
calls to writing cache.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-25 Thread Ihor Radchenko
 writes:

>> Another idea is to avoid caching of parse result for small files.
>
> I haven't been following along very closely, but seeing the
> involved complexities, I'd appreciate a knob to disable caching
> altogether.

org-element-cache-persistent

> My usage of Org hasn't triggered any slowdowns which would be
> worth all that complexity (and yes, my biggest Org file so-far
> is 36k lines, and my box isn't the fastest around).

The main reason we cannot disable org-persist is _not_ Org AST cache.
The reason is mostly caching some downloaded files.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-25 Thread Ihor Radchenko
Max Nikulin  writes:

> Ihor, I do not like that after your latest changes temporary directory 
> became world readable.

This is the only sane way for emacs -Q, AFAIK. And the cache will now
only exist while Emacs is running (for -Q cmd arg). Removed
unconditionally upon exiting.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-25 Thread Ihor Radchenko
Greg Minshall  writes:

>> Do we need to care about cleaning up /tmp?
>
> my two cents is that maybe one should not care so much about cleaning up
> /tmp, but i think it's worthwhile trying not to clutter it up too much.

I improved things a tiny bit more.
Now, whatever is created by emacs -Q will be removed as long as
`kill-emacs-hook' is executed. It is the most aggressive cleaning I can
imagine.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-23 Thread tomas
On Fri, Dec 23, 2022 at 09:12:52PM +0700, Max Nikulin wrote:

[...]

> If you demonstrate that e.g., when working with encrypted files, their
> sensitive content leaks to the cache then it will raise the severity of the
> issue. Of course, the always appreciated option is to provide a patch that
> consistently makes org-persist optional.

Understood that hint, and you are, of course, right :-)

Cheers
-- 
t


signature.asc
Description: PGP signature


Re: org-persist files in /tmp

2022-12-23 Thread Max Nikulin

On 23/12/2022 13:35, to...@tuxteam.de wrote:

On Thu, Dec 22, 2022 at 11:07:52PM +0700, Max Nikulin wrote:

Another idea is to avoid caching of parse result for small files.


I haven't been following along very closely, but seeing the
involved complexities, I'd appreciate a knob to disable caching
altogether.


Ihor wrote that org-persist usage is not limited to org-element cache. 
Another case is referenced remote files. Perhaps e.g. LaTeX preview 
still uses old approach for generated image, but it is rather close case.


I do not mind to have a setting that disables org-persist, but it 
requires some amount of work. Its priority unlikely will be high.


If you demonstrate that e.g., when working with encrypted files, their 
sensitive content leaks to the cache then it will raise the severity of 
the issue. Of course, the always appreciated option is to provide a 
patch that consistently makes org-persist optional.






Re: org-persist files in /tmp

2022-12-22 Thread tomas
On Thu, Dec 22, 2022 at 11:07:52PM +0700, Max Nikulin wrote:
> On 22/12/2022 22:45, Tim Cross wrote:
> > Could some of the issues people are concerned about regarding use of
> > /tmp be avoided if instead the temporary files were put into ~/.cache?

[...]

> Another idea is to avoid caching of parse result for small files.

I haven't been following along very closely, but seeing the
involved complexities, I'd appreciate a knob to disable caching
altogether.

My usage of Org hasn't triggered any slowdowns which would be
worth all that complexity (and yes, my biggest Org file so-far
is 36k lines, and my box isn't the fastest around).

But, of course, it's your call :-)

Cheers
-- 
t


signature.asc
Description: PGP signature


Re: org-persist files in /tmp

2022-12-22 Thread Max Nikulin

On 22/12/2022 22:45, Tim Cross wrote:

Could some of the issues people are concerned about regarding use of
/tmp be avoided if instead the temporary files were put into ~/.cache?


There is no ~/.cache on Windows, the fallback is ~/.emacs.d. org-persist 
files in ~/.emacs.d was the original source of complains. Moreover, I do 
not think emacs -q should write to the same places as emacs initialized 
in the regular way.


I see a couple of options:
- Remove the directory on exit (taking some care to not prevent quit in 
the cases of errors)
- Create the directory lazily. The variable should not be accessed 
directly, when some code is going to write a file it should call a 
function that creates the directory if it is first call.


Another idea is to avoid caching of parse result for small files.




Re: org-persist files in /tmp

2022-12-22 Thread Tim Cross


Max Nikulin  writes:

> On 22/12/2022 19:34, Ruijie Yu wrote:
>> One possible approach to this is to have all org-persist related
>> temporary directories into an overall "$TMPDIR/org-persist" directory.
>
> Predictable name in a "world" writable directory generally is not a good 
> idea. Multiple
> users may try to run Org on the same machine. There are some kernel 
> parameters to prevent
> certain type of attacks, however I am unsure concerning their default values 
> in various
> Linux distributions and what will happen if one user creates a symlink to 
> somewhere the
> under home directory of another one. So unfortunately a directory reusable by 
> different
> emacs sessions should be avoided.
>
> Ihor, I do not like that after your latest changes temporary directory became 
> world
> readable.
>
> Another point is that creating temporary files and directories must be an 
> atomic
> operation. In between of removing and recreating it an attacker might manage 
> to create a
> file with the same name.

Could some of the issues people are concerned about regarding use of
/tmp be avoided if instead the temporary files were put into ~/.cache?
To me, that would seem to be the appropriate location for such files. It
would mean that org would need to 'manage' or clean out old files, but
that shouldn't be a big issue.




Re: org-persist files in /tmp

2022-12-22 Thread Max Nikulin

On 22/12/2022 19:34, Ruijie Yu wrote:

One possible approach to this is to have all org-persist related
temporary directories into an overall "$TMPDIR/org-persist" directory.


Predictable name in a "world" writable directory generally is not a good 
idea. Multiple users may try to run Org on the same machine. There are 
some kernel parameters to prevent certain type of attacks, however I am 
unsure concerning their default values in various Linux distributions 
and what will happen if one user creates a symlink to somewhere the 
under home directory of another one. So unfortunately a directory 
reusable by different emacs sessions should be avoided.


Ihor, I do not like that after your latest changes temporary directory 
became world readable.


Another point is that creating temporary files and directories must be 
an atomic operation. In between of removing and recreating it an 
attacker might manage to create a file with the same name.





Re: org-persist files in /tmp

2022-12-22 Thread General discussions about Org-mode.


Greg Minshall  writes:

> hi, Ihor,
>
>> Do we need to care about cleaning up /tmp?
>
> my two cents is that maybe one should not care so much about cleaning up
> /tmp, but i think it's worthwhile trying not to clutter it up too much.
>
> cheers, Greg

One possible approach to this is to have all org-persist related
temporary directories into an overall "$TMPDIR/org-persist" directory.
That is, assuming that the parent directory exists, we create
org-persist temporary directories as "$TMPDIR/org-persist/XX" and
everything else would remain the same.

The downside for this approach is that, since `make-tempfile' only makes
a mkdir() call in its underlying function try_dir() assuming the
existence of its parents [1, 2], we would probably have to create the
parent via `(mkdir DIR t)' before every `make-tempfile' call within
org-persist.

[1] emacs/src/fileio.c
[2] emacs/lib/tempname.c

Best,


RY



Re: org-persist files in /tmp

2022-12-21 Thread Tim Cross


Ihor Radchenko  writes:

> "Fraga, Eric"  writes:
>
>> for some reason, I am now getting many (tens) directories of the form
>> org-persist-NN in /tmp.  These seem to include an index file and a
>> cache type sub-directory structure.  Why are these there and does
>> anything clean them up?
>>
>> I have nothing related to org-persist in my configuration that I can
>> see.
>
> If you run something like make test or emacs -Q + org, it is expected.
> These are throwaway directories used by org-persist for emacs -Q.
>
> Do we need to care about cleaning up /tmp?

Probably not - at least not on most modern Linux systems as these tend
to have a systemd task which will clean up the temp directories on
reboot. You can usually tweak the settings for systemd-tempfiles if you
want to modify when and how often temporary files are cleaned up.



Re: org-persist files in /tmp

2022-12-21 Thread Greg Minshall
hi, Ihor,

> Do we need to care about cleaning up /tmp?

my two cents is that maybe one should not care so much about cleaning up
/tmp, but i think it's worthwhile trying not to clutter it up too much.

cheers, Greg



Re: org-persist files in /tmp

2022-12-21 Thread Ihor Radchenko
William Denton  writes:

>> You should _not_ see something like
>>
>> ((:container
>>  ((index "2.7"))
>>  :persist-file "d0/5078fe-5e31-4ddb-95a0-93ceae58df0c" :associated nil 
>> :expiry never :last-access 1671637032.483552 :last-access-hr 
>> "2022-12-21T18:37:12+0300"))
>>
>> as the only contents of "index" file.
>
> I just checked my /tmp/ and I have 32 org-persist directories, and I must 
> sadly 
> report they all have index files like I shouldn't see:

Thanks for the heads-up!
Fixed on bugfix now.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e2366ac28

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-21 Thread William Denton

On 21 December 2022, Ihor Radchenko wrote:


Could be. If so, it may indicate some issue with logic. The index should
not be written if the only entry inside index file is index version
itself.

You should _not_ see something like

((:container
 ((index "2.7"))
 :persist-file "d0/5078fe-5e31-4ddb-95a0-93ceae58df0c" :associated nil :expiry never 
:last-access 1671637032.483552 :last-access-hr "2022-12-21T18:37:12+0300"))

as the only contents of "index" file.


I just checked my /tmp/ and I have 32 org-persist directories, and I must sadly 
report they all have index files like I shouldn't see:


 ((:container
   ((index "2.7"))
   :persist-file "2f/859289-dcc1-4d55-8c91-e22b4ccc92dd" :associated nil :expiry never 
:last-access 1671429856.980864 :last-access-hr  "2022-12-19T01:04:16-0500"))

Nineteen of them were made on Monday ago, and I may have rebuilt Emacs and Org 
then, and maybe restarted Emacs, but not nineteen times.


Bill

--
William Denton
https://www.miskatonic.org/
Librarian, artist and licensed private investigator.
Toronto, Canada



Re: org-persist files in /tmp

2022-12-21 Thread Ihor Radchenko
"Fraga, Eric"  writes:

> On Wednesday, 21 Dec 2022 at 15:06, Ihor Radchenko wrote:
>> If you run something like make test or emacs -Q + org, it is expected.
>> These are throwaway directories used by org-persist for emacs -Q.
>
> I am not explicitly running either of those (at the moment).
>
> I wonder if this is somehow related to native compilation or some other
> post-build processes as I have built Emacs twice from git today (and I
> always update org when I build Emacs) and these directories have
> appeared soon thereafter (I think).

Could be. If so, it may indicate some issue with logic. The index should
not be written if the only entry inside index file is index version
itself.

You should _not_ see something like

((:container
  ((index "2.7"))
  :persist-file "d0/5078fe-5e31-4ddb-95a0-93ceae58df0c" :associated nil :expiry 
never :last-access 1671637032.483552 :last-access-hr 
"2022-12-21T18:37:12+0300"))

as the only contents of "index" file.

>> Do we need to care about cleaning up /tmp?
>
> Maybe not but I do use /tmp for some things and having tens of these
> directories does clutter things up a bit.  I'm happy to ignore but was
> curious as to why they were there at all.

This has been introduced in

2944a2152d491452697cc4db538d6b2344c0e37d
Author: Ihor Radchenko 
org-persist: Use temporary index for emacs -Q

* lisp/org-persist.el (org-persist--disable-when-emacs-Q): Rename
`org-persist-disable-when-emacs-Q' to internal variable.  Update the
docstring.
(org-persist-read):
(org-persist-write):
(org-persist-gc): Do not disable persistence.  Persistence is
necessary for remote file caching to work within a single Emacs
session.  Instead, use temporary directory as index for emacs -Q.

Also, see
https://orgmode.org/list/1158097067.265983.1670026787...@mail1.libero.it

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: org-persist files in /tmp

2022-12-21 Thread Fraga, Eric
On Wednesday, 21 Dec 2022 at 15:06, Ihor Radchenko wrote:
> If you run something like make test or emacs -Q + org, it is expected.
> These are throwaway directories used by org-persist for emacs -Q.

I am not explicitly running either of those (at the moment).

I wonder if this is somehow related to native compilation or some other
post-build processes as I have built Emacs twice from git today (and I
always update org when I build Emacs) and these directories have
appeared soon thereafter (I think).

> Do we need to care about cleaning up /tmp?

Maybe not but I do use /tmp for some things and having tens of these
directories does clutter things up a bit.  I'm happy to ignore but was
curious as to why they were there at all.

Thank you,
eric

-- 
: Eric S Fraga, with org release_9.6-124-g036cc0 in Emacs 30.0.50


Re: org-persist files in /tmp

2022-12-21 Thread Ihor Radchenko
"Fraga, Eric"  writes:

> for some reason, I am now getting many (tens) directories of the form
> org-persist-NN in /tmp.  These seem to include an index file and a
> cache type sub-directory structure.  Why are these there and does
> anything clean them up?
>
> I have nothing related to org-persist in my configuration that I can
> see.

If you run something like make test or emacs -Q + org, it is expected.
These are throwaway directories used by org-persist for emacs -Q.

Do we need to care about cleaning up /tmp?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at