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: Wed, 15 Jan 2025 23:02:32 +0000

"Stefan Kangas" <[email-masked]> writes:

> Pip Cet <[email-masked]> writes:
>
>> Looks good to me.  However, on the scratch/no-purespace branch, we need
>> to ensure that this function cannot be called on the empty multibyte
>> string, turning it into a second empty unibyte string!  I'll push a fix
>> to that branch in a bit.
>
> Please do, thanks.

No need, I had forgotten the strange way STRING_SET_MULTIBYTE treats its
argument as an lvalue.  It's a minor issue, but I would prefer it if the
argument were provided as a pointer to a Lisp_Object so it's clear it
can be modified.

>> However, if this function is meant to clear a string from memory
>> to prevent leaving confidential data in memory, that won't work with
>> MPS.  (I suppose it's reasonably clear that no protection whatsoever
>> applies to string properties?)
>>
>> The problem is MPS allocates string data in a "copying" pool, so several
>> copies of the string will usually be present.
>>
>> alloc.c string compaction may also result in recently-used short strings
>> remaining in memory, IIUC, but only if an alloc.c GC cycle happens
>> between string creation and the clear-string call.  (But if it does
>> happen, it's very likely that a copy of the string is created!)
>>
>> How important is this feature?  What we have now is better that nothing,
>> but if people start relying on it in any way, that's worse than nothing.
>> A stern warning in the docstring may help reduce the problem.
>
> I think a stern warning in the docstring makes sense.  This is
> potentially security-critical, and we should better make sure that we
> can make no guarantees here.

I'm worried that providing a "security" feature like this one may raise
false expectations that Emacs provides similar "security" features
(usually, in this context, "security" very quickly becomes code for
"ensure that sensitive data is only ever exposed to
manufacturer-provided proprietary software") in other contexts, and I
really don't think we want to do that.

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; it's in the undo history; it's in the M-x
view-lossage output.  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.

The philosophical reason is that it seems harmless to offer an optional
additional security feature with no promises, but we've seen these
"evolve" into requirements which are being abused to harm Free Software,
in particular, in too many cases.  Software signing used to be optional
with all promises made it would be used only to improve security, not to
lock out Free Software, but now many people cannot run Emacs on their
locked-down employer-provided devices, and I fear it's only a matter of
time before that is expanded to most Android phones.

> It would be nice if we could have a guaranteed secure way of allocating
> and deallocating strings for passwords from ELisp.  Could something like
> that be done?  I can think of several dumb and hacky ways to do it, but
> none that are clean or smart.

We could expose pin_string: pinned strings aren't compacted, so clearing
them is safer if the old GC is in use.

But it's not clear to me what this is supposed to protect against.  If
it's physical memory inspection, we need to know a lot about the CPU for
our operation to make sense; pinning a short string and memsetting it
will virtually ensure that the previous contents will remain in RAM for
longer than they would if the string had been garbage-collected and
overwritten.

If it's disk storage, we need to look into madvise(2), mlock(2),
mlock2(), and memfd_secret(2), at least on GNU/Linux (in the case of
memfd_secret, I'm tempted to omit the "GNU/": it seems to me that this
is a Linux kernel attempt to obey security protocols which ultimately
lead to "Trusted Computing", and the GNU position on that is clear).

This isn't a simple matter, and it may be best to remove clear-string
entirely.

Pip