Back from vacation, sorry for the late reply.

On 2013年09月30日 17:29, David Laight wrote:

quoted

quoted

This is a false positive warning as the destination pointer "buf"
pointers to
an ZERO length array, which actually will occupy alg.buf mostly.
Fix this by using memcpy.

struct xfrm_algo {
          char            alg_name[64];
          unsigned int    alg_key_len;    /* in bits */
          char            alg_key[0];
};

struct {
          union {
                  struct xfrm_algo alg;
                  struct xfrm_algo_aead aead;
                  struct xfrm_algo_auth auth;
          } u;
          char buf[XFRM_ALGO_KEY_BUF_SIZE];
} alg = {};

buf = alg.u.alg.alg_key;

That is worse than horrid...
The tools have every right to complain about any accesses to alg_key[].

Only when using strcpy, because a build in checking inserted in this function.

quoted

quoted

---
   ip/xfrm_state.c |    2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index 0d98e78..5cc87d3 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c

@@ -159,7 +159,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg,

enum xfrm_attr_type_t type,
               if (len>  max)
                   invarg("\"ALGO-KEY\" makes buffer overflow\n", key);

I presume there is a return hiding in invarg().

Good guess :)

quoted

quoted

-            strncpy(buf, key, len);
+            memcpy(buf, key, len);

Passing the length of the SOURCE to strncpy() is almost always wrong.
You are still not terminating the copied string.

Don't worry.

The length using here has been increased by 1 at the beginning of the function,
so the copied string to the destination is terminated well.

	David

-- 
浮沉随浪只记今朝笑

--fan