search

HACKERALERT / Picocrypt

A very small, very simple, yet very secure encryption tool.
GNU General Public License v3.0
2.42k stars 145 forks source link

[BUG] Memory overusage, memory leak #152

Closed hakavlad closed 1 year ago

hakavlad commented 1 year ago

Host: Debian 11. Running the latest Picocrypt.AppImage.

At the start picocrypt uses 2 MiB memory (VmRSS). After encrypting one file: 1 GiB. After encrypting the second file: 2 GiB VmRSS. Seems like picocrypt does not clear argon output from its memory.

$ cat /proc/578260/status
Name:   picocrypt
Umask:  0022
State:  S (sleeping)
Tgid:   578260
Ngid:   0
Pid:    578260
PPid:   578259
TracerPid:  0
Uid:    1000    1000    1000    1000
Gid:    1000    1000    1000    1000
FDSize: 1024
Groups: 24 25 27 29 30 44 46 109 112 998 1000 
NStgid: 578260
NSpid:  578260
NSpgid: 1434
NSsid:  1434
VmPeak:  5380060 kB
VmSize:  5380060 kB
VmLck:         0 kB
VmPin:         0 kB
VmHWM:   2292980 kB
VmRSS:   2292980 kB                 # 2 GiB+
RssAnon:     2227548 kB         # 2 GiB+ memory usage
RssFile:       64792 kB
RssShmem:        640 kB
VmData:  2541200 kB
VmStk:       132 kB
VmExe:      3848 kB
VmLib:    129640 kB
VmPTE:      5020 kB
VmSwap:        0 kB
HugetlbPages:          0 kB

Upd: Memory usage returned to 2MB after some time of inactivity.

Suggestion: add warning to README.md: picocrypt uses up to 2 GB of memory. In some situations, the use of picocrypt can lead to Out of Memory.

Expected Behavior: Memory usage drops to 2 MB immediately after KDF execution finishes.

HACKERALERT commented 1 year ago

Yeah as you've mentioned, the memory will be freed eventually. I'm pretty sure this is a Go garbage collector thing where it frees the memory internally but doesn't return it to the system immediately to reduce expensive memory operations. Using debug.FreeOSMemory() will free the memory usage immediately, but I don't think using the debug package is a good idea in production-level software. On a low-memory system, I've found experimentally that the extra memory usage for subsequent Argon2 operations doesn't seem to affect anything, the memory that is internally freed is just reused. So I think it's okay to assume that the average user has at least 2 GB memory since anything below that and the limiting factor would be Argon2 running on a slow CPU which would make it impossible to run Argon2 in quick succession anyways. Most systems come with at least 8 GB these days so I don't think it's necessary to warn the user. Thanks for reporting!

hakavlad commented 1 year ago

OK thanks.