Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some small fixes and possible valuable information #291

Closed
wants to merge 12 commits into from

Conversation

alexander-pick
Copy link
Contributor

Description

I was hunting down a global oobr triggered in kasan. Tuned out the modprobe has to be run from sudo -s instead just using sudo only to make it work. The only noticeable trace is the error dumping to dmesg if it fails. I assume this happens if kptr_restrict is set and the user who runs modprobe has no CP_SYSLOG permissions.

During the process I made a few changes which might be valuable:

  • reflected the above situation to README
  • added some lines to remove static keywords add-exports.sh, otherwise my 6.x kernel refused to compile (using clang)
  • added some checks to avoid hashes of zero sized sections so it's visible in debug log if this happens by setting the hash to 0xFFFFFFFF
  • fixed CONFIG_SECURITY_SELINUX vs. CONFIG_SECURITY_SELINUX_DEVELOP which would enable sel_write_enforce
  • fixes a typo in P_LKRG_EXPLOIT_DETECTION_SECURTIY_PTRACE_ACCESS_H
  • fixed some types for clarification
  • removed a sizeof as we can safely assume 8 for uint64_t and the next line shares this assumption by using & 7
  • added a case 0 to the hash algorithm to satisfy clang and avoid warnings
  • clarified a warning which I found to be misleading (I build in-tree, but as a module)

How Has This Been Tested?

Tested the build a lot of times on multiple systems with different configuration

Your Name and others added 5 commits October 9, 2023 18:01
* fixed typo in include guard
* fixed some other oob read and added a warning to debug output
* clarified types in signature code
* clarified readme as "sudo modprobe ..." will interfer with kptr_restrict if user has no CP_SYSLOG
README Outdated Show resolved Hide resolved
sed -i 's/static int change_page_attr_set_clr/int change_page_attr_set_clr/g' arch/x86/mm/pat/set_memory.c
sed -i 's/static struct dentry \*lookup_fast/struct dentry \*lookup_fast/g' fs/namei.c
sed -i 's/static long do_seccomp/long do_seccomp/g' kernel/seccomp.c
sed -i 's/static void __seccomp_filter_release(/void __seccomp_filter_release(/g' kernel/seccomp.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already set at line 9 of this script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My additions remove the static keywords, line 9 adds the export. If static remains the module will fail to compile properly:

ERROR: modpost: vmlinux: local symbol 'change_page_attr_set_clr' was exported
ERROR: modpost: vmlinux: local symbol '__put_seccomp_filter' was exported
ERROR: modpost: vmlinux: local symbol 'do_seccomp' was exported
ERROR: modpost: vmlinux: local symbol 'lookup_fast' was exported

Essentially my changes stop this from happening.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks. Can you group the symbols together? e.g., when operating on __seccomp_filter_release we likely want to do all the changes at once and not in 2-3 different places. I assume, your changes can me merged into the original lines manipulating on that specific symbol. For other symbols just to work on them at the same place in file.

@@ -8,3 +8,9 @@ echo "EXPORT_SYMBOL(lookup_fast);" >> fs/namei.c
# keep symbol export inside the ifdef block defining the symbol
sed -i '/static void __seccomp_filter_release.*/iEXPORT_SYMBOL(__put_seccomp_filter);\n' kernel/seccomp.c
echo "EXPORT_SYMBOL(do_seccomp);" >> kernel/seccomp.c
# fixes modpost errors
sed -i 's/static int change_page_attr_set_clr/int change_page_attr_set_clr/g' arch/x86/mm/pat/set_memory.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already export that symbol, why do you need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, it fixes modpost errors due to local syms.

@@ -8,3 +8,9 @@ echo "EXPORT_SYMBOL(lookup_fast);" >> fs/namei.c
# keep symbol export inside the ifdef block defining the symbol
sed -i '/static void __seccomp_filter_release.*/iEXPORT_SYMBOL(__put_seccomp_filter);\n' kernel/seccomp.c
echo "EXPORT_SYMBOL(do_seccomp);" >> kernel/seccomp.c
# fixes modpost errors
sed -i 's/static int change_page_attr_set_clr/int change_page_attr_set_clr/g' arch/x86/mm/pat/set_memory.c
sed -i 's/static struct dentry \*lookup_fast/struct dentry \*lookup_fast/g' fs/namei.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also exported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, it removes the static keyword, not adds an export, it's just to solve modpost errors.

# fixes modpost errors
sed -i 's/static int change_page_attr_set_clr/int change_page_attr_set_clr/g' arch/x86/mm/pat/set_memory.c
sed -i 's/static struct dentry \*lookup_fast/struct dentry \*lookup_fast/g' fs/namei.c
sed -i 's/static long do_seccomp/long do_seccomp/g' kernel/seccomp.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

@@ -26,11 +26,11 @@ uint128_t p_global_siphash_key;
inline void p_lkrg_siphash(const uint8_t *in, const size_t inlen, const uint8_t *k,
uint8_t *out, const size_t outlen);

notrace uint64_t p_lkrg_fast_hash(const char *p_data, unsigned int p_len) {
notrace uint64_t p_lkrg_fast_hash(const unsigned char *p_data, unsigned int p_len) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All code paths I found pass an unsigned char* (even casted to one) not a char* so corrected this to match the inputs. In the end it won't matter as the pointer is a pointer but I cam across it a few times looking through the code.

src/modules/hashing/p_lkrg_fast_hash.c Outdated Show resolved Hide resolved
@@ -65,25 +65,25 @@ notrace inline void p_lkrg_siphash(const uint8_t *in, const size_t inlen, const

switch (left) {
case 7:
b |= ((uint64_t)in[6]) << 48;
b |= ((uint64_t)end[6]) << 48;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? looks wrong, CC: @solardiz

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All implementations I know use end not in, but once in reached = end in the for loop it should be the same. That was just changed for readability and to avoid a moving pointer.

Copy link
Contributor Author

@alexander-pick alexander-pick Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just reevaluated it a few times. in and end are equal after the loop, as in get bumped again but not processed by the for. So writing either end or in is pretty much the same. My reference for using end was mainly the Android / Linux kernel version here:

https://android.googlesource.com/kernel/common/+/3af7a2f61023/lib/siphash.c
https://github.com/torvalds/linux/blob/master/lib/siphash.c

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave it to @solardiz as he is more competent in that field

@@ -78,6 +78,6 @@ typedef struct uint128_t {

extern uint128_t p_global_siphash_key;

uint64_t p_lkrg_fast_hash(const char *data, unsigned int len);
uint64_t p_lkrg_fast_hash(const unsigned char *data, unsigned int len);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the comment for the .c:

All code paths I found pass an unsigned char* (even casted to one) not a char* so corrected this to match the inputs. In the end it won't matter as the pointer is a pointer but I cam across it a few times looking through the code.

@@ -137,6 +137,8 @@
case P_LOG_FLOOD: \
p_print_ret = printk(KERN_DEBUG P_LKRG_SIGNATURE "FLOOD: " p_fmt "\n", ## p_args); \
break; \
default: \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's needed since we manually inject specific type of log. If we decide to 'default' anyway, it should be an invasive break not silent break. E.g., it should include #error macro breaking compilation. @solardiz what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang will shelf out a warning if you have no default in there. As I compile in-tree with CONFIG_WERROR the build will fail and I get a lot of warnings from clang about it. Other than that it doesn\t do much at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add more context for that 'default' statement per my original comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adam, I think we can add a default: to pacify compiler warnings. We just need to think where exactly to put it best. We can't do a #error since this would only be detected after the preprocessor stage (even if still at compile-time). We could maybe do a static_assert, but I think this is too minor a use case to start using it in this codebase.

Copy link
Contributor Author

@alexander-pick alexander-pick Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a snippet with some warning clang will give you for each p_print_log as reference. As clang being a pretty wide spread compiler these days I think it is important to deal with it. Technically you can alternatively add a case 0 but default caches other undefined settings as well.

In file included from /work/kernel/lkrg/src/modules/database/arch/arm/p_arm_metadata.c:29:
In file included from /work/kernel/lkrg/src/modules/database/arch/arm/../../../../p_lkrg_main.h:435:
In file included from /work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/exploit_detection/p_exploit_detection.h:583:
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/exploit_detection/arch/x86/p_ed_x86_arch.h:186:25: warning: no case matching constant switch condition '0'
            p_print_log(P_LOG_ALERT, "BLOCK: CPU: SMAP got disabled, re-enabling");
                        ^~~~~~~~~~~
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/print_log/p_lkrg_print_log.h:92:21: note: expanded from macro 'P_LOG_ALERT'
#define P_LOG_ALERT 0
                    ^
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/print_log/p_lkrg_print_log.h:112:12: note: expanded from macro 'p_print_log'
   switch (p_level) {                                                                      \
           ^~~~~~~
In file included from /work/kernel/lkrg/src/modules/database/arch/arm/p_arm_metadata.c:29:
In file included from /work/kernel/lkrg/src/modules/database/arch/arm/../../../../p_lkrg_main.h:435:
In file included from /work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/exploit_detection/p_exploit_detection.h:583:
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/exploit_detection/arch/x86/p_ed_x86_arch.h:199:25: warning: no case matching constant switch condition '0'
            p_print_log(P_LOG_ALERT, "ALLOW: CPU: SMAP got disabled, accepting");
                        ^~~~~~~~~~~
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/print_log/p_lkrg_print_log.h:92:21: note: expanded from macro 'P_LOG_ALERT'
#define P_LOG_ALERT 0
                    ^
/work/kernel/lkrg/src/modules/database/arch/arm/../../../../modules/print_log/p_lkrg_print_log.h:112:12: note: expanded from macro 'p_print_log'
   switch (p_level) {                                                                      \
           ^~~~~~~
15 warnings generated.

@alexander-pick
Copy link
Contributor Author

Is there anything else I could help with in regards of this PR? Were the comments helpful so far?

Your Name added 2 commits October 21, 2023 18:33
some cosmetics
fixed clang warnign with optimize
flush_workqueue warning fixed, not optimal but better than werror for now
@solardiz
Copy link
Contributor

Hi. Thank you @alexander-pick for contributing. I'm sorry I don't yet comment on the individual issues. I think that this PR as a whole is not to be merged(*), but there's valuable information here and many of the individual changes should get in, some in revised form.

(*) too many assorted changes, some are questionable, the separation into commits is historical rather than logical, commit author names are placeholders, there are merge commits instead of rebases

@Adam-pi3
Copy link
Collaborator

I just replied to some of the comments. Sorry for delay - had busy week.

@solardiz
Copy link
Contributor

Tuned out the modprobe has to be run from sudo -s instead just using sudo only to make it work. The only noticeable trace is the error dumping to dmesg if it fails. I assume this happens if kptr_restrict is set and the user who runs modprobe has no CP_SYSLOG permissions.

This makes no sense to me, but perhaps I am missing something. First, why would these different ways to use sudo affect availability of CAP_SYSLOG to modprobe? Then, why would CAP_SYSLOG matter here? Finally, what's the presumably relevant change here - is it not passing the command on sudo command-line, or is it using the -s option?

This looks like a maybe-workaround for some issue (or non-issue) that probably remains. Is this your understanding, too?

If so, perhaps this isn't a reliable workaround either. I really doubt we want to have it, and certainly we don't want to have the weird references to these guessed details in LKRG messages.

Copy link
Contributor

@solardiz solardiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made some comments. Overall, I think instead of this PR with a lot in it, we need separate PRs (initially just one, then more) for the different kinds of changes - e.g., start with fixing the typos and changing wording of messages and using proper committer name there - we'll merge that, then proceed to the next PR with slightly less trivial changes, etc. Eventually, we'd have decided on these all (accepted some, revised others, rejected the rest). Thanks!

flush_workqueue(system_unbound_wq);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the change from flush_workqueue to __flush_workqueue needed, and why only on 6.6+? Is this possibly to workaround the warning? If so, that's not 6.6+ specific, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flushing the global workqueue will trigger a warning now. The discussion about it is from 2022 and it was introduced with 6.6rc1.

Reference:
torvalds/linux@20bdeda
https://github.com/torvalds/linux/blob/d88520ad73b79e71e3ddf08de335b8520ae41c5c/include/linux/workqueue.h#L619

@@ -28,7 +28,7 @@ static struct kretprobe p_lkrg_dummy_kretprobe = {
.entry_handler = p_lkrg_dummy_entry,
};

__attribute__((optimize(0)))
__attribute__ ((optnone))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the change from attribute__((optimize(0))) to __attribute__ ((optnone))?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag will be cause an unknown attribute warning with clang, optnone works. Possibly it needs to be changed to for full compatibility:

#ifdef __clang__
__attribute__ ((optnone))
#endif
#ifdef __GNUC__
__attribute__((optimize(0)))
#endif

@@ -137,6 +137,8 @@
case P_LOG_FLOOD: \
p_print_ret = printk(KERN_DEBUG P_LKRG_SIGNATURE "FLOOD: " p_fmt "\n", ## p_args); \
break; \
default: \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adam, I think we can add a default: to pacify compiler warnings. We just need to think where exactly to put it best. We can't do a #error since this would only be detected after the preprocessor stage (even if still at compile-time). We could maybe do a static_assert, but I think this is too minor a use case to start using it in this codebase.

@solardiz
Copy link
Contributor

For supporting zero-sized sections, I think we need to revise the hash function itself instead of its call sites. This also side-steps the question of which sections can or cannot be zero-size.

@solardiz
Copy link
Contributor

added some checks to avoid hashes of zero sized sections so it's visible in debug log if this happens by setting the hash to 0xFFFFFFFF

What's the problem and goal of change here? Did anything misbehave at all? Or is it simply an enhancement to be able to see which sections are zero-sized? If so, why is that desirable?

@@ -162,7 +162,7 @@ static const struct p_functions_hooks {
"Won't enforce validation on 'generic_permission'",
1
},
#ifdef CONFIG_SECURITY_SELINUX
#ifdef CONFIG_SECURITY_SELINUX_DEVELOP
{ "sel_write_enforce",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that this change from CONFIG_SECURITY_SELINUX to CONFIG_SECURITY_SELINUX_DEVELOP is correct, we also need to make it in src/modules/exploit_detection/syscalls/p_sel_write_enforce/p_sel_write_enforce.c. Otherwise we'll have dead code in some builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see it here that CONFIG_SECURITY_SELINUX_DEVELOP is the correct flag enabling this function:

Line 122:
https://github.com/torvalds/linux/blob/master/security/selinux/include/security.h

Good point about the dead code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that CONFIG_SECURITY_SELINUX_DEVELOP is the correct flag to check, but the source code link in the comment above isn't for sel_write_enforce. Here's the correct snippet, from git blame security/selinux/selinuxfs.c:

^1da177e4c3f4 (Linus Torvalds          2005-04-16 15:20:36 -0700  134) #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
1872981b51dac (Eric Paris              2008-04-17 14:15:45 -0400  135) static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
^1da177e4c3f4 (Linus Torvalds          2005-04-16 15:20:36 -0700  136)                           size_t count, loff_t *ppos)

I used git blame to ensure there was no recent change - in other words, that this check would be valid across our range of supported kernels.

(unsigned int)p_db.kernel_ex_table.p_size);
} else {
p_debug_log(P_LOG_DEBUG,
"hashing of _ex_table failed, is kptr_restrict set or CAP_SYSLOG missing?");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would kptr_restrict set or CAP_SYSLOG missing result in sections appearing to be zero-sized? This really makes no sense to me, in any of these places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is what I see with sudo only, works with sudo -s:

cat /proc/kallsyms | grep rodata
0000000000000000 T mark_rodata_ro
0000000000000000 D __start_rodata
0000000000000000 D rodata_enabled
0000000000000000 R __end_rodata
0000000000000000 R __end_rodata_aligned
0000000000000000 R __end_rodata_hpage_align
0000000000000000 d rodata_resource
0000000000000000 t set_debug_rodata
0000000000000000 d __setup_str_set_debug_rodata
0000000000000000 d __setup_set_debug_rodata
0000000000000000 t hash_from_kernel_rodata	[lkrg]

I assumed kptr_restrict is the issue here as the module also had some issues. I didn't debug it in detail, another possible issue might be some issue with the kprobe hack used to get get_kallsyms_address. So I basically added some input validation to avoid a null pointer. The message is just info I added to see if something went bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@solardiz
Copy link
Contributor

@alexander-pick First of all, thank you for your effort. As I already mentioned, this PR as a whole is not merge-able. Do you want to start creating separate PRs for the different kinds of changes here, or should I or Adam be creating our own PRs based on your suggested changes here and crediting you indirectly (e.g., as Suggested-by or Co-authored-by, perhaps with your review)? Thanks again.

@alexander-pick
Copy link
Contributor Author

No problem, you can do it as you want and credit me in some way as you feel it's best done. The PR originated in work I did for myself and some review I did for curiosity, that is why it sadly got a bit mixed up. But I felt like some of it might be valuable and hope it helps.

@solardiz
Copy link
Contributor

No problem, you can do it as you want and credit me in some way as you feel it's best done.

I've proceeded to take pieces from this PR into separate commits and am testing them in my fork of the repo. I use Co-authored-by: Alexander Pick <[email protected]>.

@solardiz
Copy link
Contributor

I've proceeded to take pieces from this PR into separate commits and am testing them in my fork of the repo.

My commits so far passed tests, so pushed to this repo as well. There are still many more things to take from this PR.

solardiz added a commit that referenced this pull request Sep 19, 2024
See #291

Co-authored-by: Alexander Pick <[email protected]>
solardiz added a commit to solardiz/lkrg that referenced this pull request Sep 20, 2024
See lkrg-org#291 although it doesn't make clear whether any such clashes were
observed yet.

Co-authored-by: Alexander Pick <[email protected]>
solardiz added a commit to solardiz/lkrg that referenced this pull request Sep 20, 2024
@solardiz
Copy link
Contributor

For SipHash, I see several kinds of changes here:

  1. Use Linux's own implementation where available.
  2. Avoid possible clash on uint128_t.
  3. Avoid compiler warnings.
  4. Make the code more similar to other implementations.

Regarding 1, I am not sure. On one hand, it's good to make LKRG smaller (in terms of compiled code) and use what's maintained upstream. On the other, we'll need to maintain and test our own implementation anyway (for older kernels) and using the kernel's may go against our security model. We compare those hashes in case the kernel or a module are modified - to hopefully detect that and respond to it. If we use the kernel's SipHash, we trust it, including with the per-boot random key passed to it as a function argument. So we'd possibly make it a bit easier for a kernel rootkit or such to be undetected by LKRG, as it could easily intercept SipHash and compute/return updated hashes that would be valid for the key we use. They could probably intercept LKRG's SipHash just as well, but at least that's something we should defend against in future versions of LKRG (make it harder to locate). For a kernel's exported symbol, we would have no reasonable recourse other than stopping to use it.

For 2, I'm merging this change (simplified).

For 3, I'm adding an #if to allow building this code in userspace, which enabled me to easily expose and then avoid some compiler warnings with gcc and clang.

For 4, I am not sure. I'm not doing anything yet. If we're to clean up the code, we could as well make even more clean-ups. For example, if/when we switch to Linux kernel's coding style, we could maybe have the code almost match what's used there, including indentation.

@solardiz
Copy link
Contributor

For supporting zero-sized sections, I think we need to revise the hash function itself instead of its call sites.

Actually, I see nothing to revise - it appears to work just fine on empty input as-is:

#include <stdio.h>
#include <stdint.h>
#include "p_lkrg_fast_hash.h"

int main(void)
{
	printf("%llx\n", (unsigned long long)p_lkrg_fast_hash(NULL, 0));
	return 0;
}

prints 1e924b9d737700d7, so that's apparently the hash value for empty input with all-zero key (not initialized here).

@alexander-pick Can you please explain why you wanted to handle zero-sized sections specially? I see currently no reason, but maybe I'm missing something.

@solardiz
Copy link
Contributor

prints 1e924b9d737700d7, so that's apparently the hash value for empty input with all-zero key (not initialized here).

Searching for this value finds it in https://github.com/havenwood/digest-sip_hash so either it's correct (I hope) or we have a common bug. ;-)

solardiz added a commit to solardiz/lkrg that referenced this pull request Sep 20, 2024
@solardiz
Copy link
Contributor

@alexander-pick I think I'm done revising and merging what was needed from this PR. Can you please take a look and test the current revision (main branch in this repo) and confirm if it's OK to close this PR as completed? If you're still getting any compiler warnings, please let us know. Thank you!

@solardiz solardiz closed this Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants