Subject: bug#75581: 30.0.93; Crash when copy-sequence on clear-string'ed multibyte strings with text properties

From: Pip Cet <[email-masked]>
Date: Thu, 16 Jan 2025 11:40:16 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Cc: [email-masked], Kanakana <[email-masked]>,
>>  Andreas Schwab <[email-masked]>
>> Date: Wed, 15 Jan 2025 23:02:32 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> The technical reason is that we simply cannot guarantee a string which
>> is entered in the minibuffer or in a real buffer won't be visible
>> somewhere: if it's in a buffer, it may be in the gap, and AFAIK there's
>> no way to clear that
>
> compact_buffer does at least part of that job, AFAICT, if it shrinks
> the gap.

I don't see how.  Worst case, it calls xrealloc which often leaves an
extra copy in memory.

> And it should be easy to add a new function that clears the
> gap completely, and expose it to Lisp or call it from clear-string.

It's easy to clear the current gap completely, but it's not what people
want, which is to actually remove the sensitive data from memory.  That
is much, much harder.

>> it's in the undo history; it's in the M-x view-lossage output.
>
> We could handle that as well, by clearing the undo-list and
> recent-keys.

So we're up to five necessary changes to do what the callers of this
function apparently assume will be achieved by it:

1. realloc must be hardened to clear the old memory
2. the gap must be cleared
3. undo history must be cleared
4. recent-keys must be cleared
5. string compaction must be hardened not to leave extra copies of
strings around

The cache line problem still remains, of course.

None of this is easy.

>> Physically, clearing memory only makes sense if
>> it's an entire cache line: a partial cache line memset will leave the
>> original copy in RAM while creating a write-back entry in the CPU cache:
>> if that is what you're worried about, the naive approach will actually
>> make things worse.
>
> I think there are APIs to clear memory securely, no?

Probably.  Even getting to the point where that matters would require
too many changes, though.

> Removing this function sounds too extreme to me, since we have that
> for many years.

You're probably right, but it should be clearly documented to prevent
only ordinary Lisp code from accessing the password, and not to do any
of the five things above.

read-passwd, for example, definitely needs more warnings than it
currently has: if I dump core after evaluating (clear-string (prog1
(read-passwd "") (garbage-collect))), I see at least three copies of the
password in the coredump.  (Possibly more that use fixnums).

Pip