Re: [PATCH 04/13] lib/base64: RFC4648-compliant base64 encoding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/10/21 9:13 PM, Eric Biggers wrote:
On Tue, Aug 10, 2021 at 02:42:21PM +0200, Hannes Reinecke wrote:
Add RFC4648-compliant base64 encoding and decoding routines.

Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
---
  include/linux/base64.h |  16 ++++++
  lib/Makefile           |   2 +-
  lib/base64.c           | 115 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 132 insertions(+), 1 deletion(-)
  create mode 100644 include/linux/base64.h
  create mode 100644 lib/base64.c

diff --git a/include/linux/base64.h b/include/linux/base64.h
new file mode 100644
index 000000000000..660d4cb1ef31
--- /dev/null
+++ b/include/linux/base64.h
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * base64 encoding, lifted from fs/crypto/fname.c.
+ */

As I mentioned previously, please make it very clear which variant of Base64 it
is (including whether padding is included and required or not), and update all
the comments accordingly.  I've done so in fs/crypto/fname.c with
https://lkml.kernel.org/r/20210718000125.59701-1-ebiggers@xxxxxxxxxx.  It would
probably be best to start over with a copy of that and modify it accordingly to
implement the desired variant of Base64.

Hmm. Looking into it.

+/**
+ * base64_decode() - base64-decode some bytes
+ * @src: the base64-encoded string to decode
+ * @len: number of bytes to decode
+ * @dst: (output) the decoded bytes.

"@len: number of bytes to decode" is ambiguous as it could refer to either @src
or @dst.  I've fixed this in the fs/crypto/fname.c version.


Ok, will be updating.

+ *
+ * Decodes the base64-encoded bytes @src according to RFC 4648.
+ *
+ * Return: number of decoded bytes
+ */

Shouldn't this return an error if the string is invalid?  Again, see the latest
fs/crypto/fname.c version.


Indeed, it should. Will be fixing it up.

+int base64_decode(const char *src, int len, u8 *dst)
+{
+        int i, bits = 0, pad = 0;
+        u32 ac = 0;
+        size_t dst_len = 0;
+
+        for (i = 0; i < len; i++) {
+                int c, p = -1;
+
+                if (src[i] == '=') {
+                        pad++;
+                        if (i + 1 < len && src[i + 1] == '=')
+                                pad++;
+                        break;
+                }
+                for (c = 0; c < strlen(lookup_table); c++) {

strlen() shouldn't be used in a loop condition like this.

Why not?
'lookup_table' is pretty much static, so where't the problem?

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux