Hi Jacob,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v5.0-rc2 next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:    
https://github.com/0day-ci/linux/commits/Jacob-Wen/net-l2tp-fix-reading-optional-fields/20190122-193040
config: x86_64-randconfig-x008-201903 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-14) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   net/l2tp/l2tp_core.c: In function 'l2tp_recv_common':
>> net/l2tp/l2tp_core.c:673:3: warning: ISO C90 forbids mixed declarations and 
>> code [-Wdeclaration-after-statement]
      u32 l2h = ntohl(*(__be32 *) ptr);
      ^~~

vim +673 net/l2tp/l2tp_core.c

b6dc01a4 James Chapman          2013-07-02  560  
f7faffa3 James Chapman          2010-04-02  561  /* Do receive processing of 
L2TP data frames. We handle both L2TPv2
f7faffa3 James Chapman          2010-04-02  562   * and L2TPv3 data frames here.
f7faffa3 James Chapman          2010-04-02  563   *
f7faffa3 James Chapman          2010-04-02  564   * L2TPv2 Data Message Header
f7faffa3 James Chapman          2010-04-02  565   *
f7faffa3 James Chapman          2010-04-02  566   *  0                   1      
             2                   3
f7faffa3 James Chapman          2010-04-02  567   *  0 1 2 3 4 5 6 7 8 9 0 1 2 
3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3 James Chapman          2010-04-02  568   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  569   * |T|L|x|x|S|x|O|P|x|x|x|x|  
Ver  |          Length (opt)         |
f7faffa3 James Chapman          2010-04-02  570   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  571   * |           Tunnel ID       
    |           Session ID          |
f7faffa3 James Chapman          2010-04-02  572   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  573   * |             Ns (opt)      
    |             Nr (opt)          |
f7faffa3 James Chapman          2010-04-02  574   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  575   * |      Offset Size (opt)    
    |    Offset pad... (opt)
f7faffa3 James Chapman          2010-04-02  576   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  577   *
f7faffa3 James Chapman          2010-04-02  578   * Data frames are marked by 
T=0. All other fields are the same as
f7faffa3 James Chapman          2010-04-02  579   * those in L2TP control 
frames.
f7faffa3 James Chapman          2010-04-02  580   *
f7faffa3 James Chapman          2010-04-02  581   * L2TPv3 Data Message Header
f7faffa3 James Chapman          2010-04-02  582   *
f7faffa3 James Chapman          2010-04-02  583   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  584   * |                      L2TP 
Session Header                      |
f7faffa3 James Chapman          2010-04-02  585   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  586   * |                      
L2-Specific Sublayer                     |
f7faffa3 James Chapman          2010-04-02  587   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  588   * |                        
Tunnel Payload                      ...
f7faffa3 James Chapman          2010-04-02  589   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  590   *
f7faffa3 James Chapman          2010-04-02  591   * L2TPv3 Session Header Over 
IP
f7faffa3 James Chapman          2010-04-02  592   *
f7faffa3 James Chapman          2010-04-02  593   *  0                   1      
             2                   3
f7faffa3 James Chapman          2010-04-02  594   *  0 1 2 3 4 5 6 7 8 9 0 1 2 
3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3 James Chapman          2010-04-02  595   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  596   * |                           
Session ID                          |
f7faffa3 James Chapman          2010-04-02  597   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  598   * |               Cookie 
(optional, maximum 64 bits)...
f7faffa3 James Chapman          2010-04-02  599   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  600   *                             
                                    |
f7faffa3 James Chapman          2010-04-02  601   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  602   *
f7faffa3 James Chapman          2010-04-02  603   * L2TPv3 L2-Specific Sublayer 
Format
f7faffa3 James Chapman          2010-04-02  604   *
f7faffa3 James Chapman          2010-04-02  605   *  0                   1      
             2                   3
f7faffa3 James Chapman          2010-04-02  606   *  0 1 2 3 4 5 6 7 8 9 0 1 2 
3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3 James Chapman          2010-04-02  607   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  608   * |x|S|x|x|x|x|x|x|           
   Sequence Number                  |
f7faffa3 James Chapman          2010-04-02  609   * 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3 James Chapman          2010-04-02  610   *
23fe846f Guillaume Nault        2018-01-05  611   * Cookie value and sublayer 
format are negotiated with the peer when
23fe846f Guillaume Nault        2018-01-05  612   * the session is set up. 
Unlike L2TPv2, we do not need to parse the
23fe846f Guillaume Nault        2018-01-05  613   * packet header to determine 
if optional fields are present.
f7faffa3 James Chapman          2010-04-02  614   *
f7faffa3 James Chapman          2010-04-02  615   * Caller must already have 
parsed the frame and determined that it is
f7faffa3 James Chapman          2010-04-02  616   * a data (not control) frame 
before coming here. Fields up to the
f7faffa3 James Chapman          2010-04-02  617   * session-id have already 
been parsed and ptr points to the data
f7faffa3 James Chapman          2010-04-02  618   * after the session-id.
fd558d18 James Chapman          2010-04-02  619   */
f7faffa3 James Chapman          2010-04-02  620  void l2tp_recv_common(struct 
l2tp_session *session, struct sk_buff *skb,
f7faffa3 James Chapman          2010-04-02  621                       unsigned 
char *ptr, unsigned char *optr, u16 hdrflags,
2b139e6b Guillaume Nault        2018-07-25  622                       int 
length)
fd558d18 James Chapman          2010-04-02  623  {
f7faffa3 James Chapman          2010-04-02  624         struct l2tp_tunnel 
*tunnel = session->tunnel;
fd558d18 James Chapman          2010-04-02  625         int offset;
f7faffa3 James Chapman          2010-04-02  626         u32 ns, nr;
fd558d18 James Chapman          2010-04-02  627  
f7faffa3 James Chapman          2010-04-02  628         /* Parse and check 
optional cookie */
f7faffa3 James Chapman          2010-04-02  629         if 
(session->peer_cookie_len > 0) {
31c773ec Jacob Wen              2019-01-22  630                 if 
(!pskb_may_pull(skb, ptr - optr + session->peer_cookie_len))
31c773ec Jacob Wen              2019-01-22  631                         goto 
discard;
f7faffa3 James Chapman          2010-04-02  632                 if (memcmp(ptr, 
&session->peer_cookie[0], session->peer_cookie_len)) {
a4ca44fa Joe Perches            2012-05-16  633                         
l2tp_info(tunnel, L2TP_MSG_DATA,
f7faffa3 James Chapman          2010-04-02  634                                 
  "%s: cookie mismatch (%u/%u). Discarding.\n",
a4ca44fa Joe Perches            2012-05-16  635                                 
  tunnel->name, tunnel->tunnel_id,
a4ca44fa Joe Perches            2012-05-16  636                                 
  session->session_id);
7b7c0719 Tom Parkin             2013-03-19  637                         
atomic_long_inc(&session->stats.rx_cookie_discards);
f7faffa3 James Chapman          2010-04-02  638                         goto 
discard;
f7faffa3 James Chapman          2010-04-02  639                 }
f7faffa3 James Chapman          2010-04-02  640                 ptr += 
session->peer_cookie_len;
f7faffa3 James Chapman          2010-04-02  641         }
f7faffa3 James Chapman          2010-04-02  642  
fd558d18 James Chapman          2010-04-02  643         /* Handle the optional 
sequence numbers. Sequence numbers are
fd558d18 James Chapman          2010-04-02  644          * in different places 
for L2TPv2 and L2TPv3.
fd558d18 James Chapman          2010-04-02  645          *
fd558d18 James Chapman          2010-04-02  646          * If we are the LAC, 
enable/disable sequence numbers under
fd558d18 James Chapman          2010-04-02  647          * the control of the 
LNS.  If no sequence numbers present but
fd558d18 James Chapman          2010-04-02  648          * we were expecting 
them, discard frame.
fd558d18 James Chapman          2010-04-02  649          */
fd558d18 James Chapman          2010-04-02  650         ns = nr = 0;
fd558d18 James Chapman          2010-04-02  651         
L2TP_SKB_CB(skb)->has_seq = 0;
f7faffa3 James Chapman          2010-04-02  652         if (tunnel->version == 
L2TP_HDR_VER_2) {
fd558d18 James Chapman          2010-04-02  653                 if (hdrflags & 
L2TP_HDRFLAG_S) {
31c773ec Jacob Wen              2019-01-22  654                         if 
(!pskb_may_pull(skb, ptr - optr + 4))
31c773ec Jacob Wen              2019-01-22  655                                 
goto discard;
f7faffa3 James Chapman          2010-04-02  656                         ns = 
ntohs(*(__be16 *) ptr);
fd558d18 James Chapman          2010-04-02  657                         ptr += 
2;
fd558d18 James Chapman          2010-04-02  658                         nr = 
ntohs(*(__be16 *) ptr);
fd558d18 James Chapman          2010-04-02  659                         ptr += 
2;
fd558d18 James Chapman          2010-04-02  660  
fd558d18 James Chapman          2010-04-02  661                         /* 
Store L2TP info in the skb */
fd558d18 James Chapman          2010-04-02  662                         
L2TP_SKB_CB(skb)->ns = ns;
fd558d18 James Chapman          2010-04-02  663                         
L2TP_SKB_CB(skb)->has_seq = 1;
fd558d18 James Chapman          2010-04-02  664  
a4ca44fa Joe Perches            2012-05-16  665                         
l2tp_dbg(session, L2TP_MSG_SEQ,
f7faffa3 James Chapman          2010-04-02  666                                 
 "%s: recv data ns=%u, nr=%u, session nr=%u\n",
fd558d18 James Chapman          2010-04-02  667                                 
 session->name, ns, nr, session->nr);
fd558d18 James Chapman          2010-04-02  668                 }
f7faffa3 James Chapman          2010-04-02  669         } else if 
(session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
31c773ec Jacob Wen              2019-01-22  670                 if 
(!pskb_may_pull(skb, ptr - optr + 4))
31c773ec Jacob Wen              2019-01-22  671                         goto 
discard;
31c773ec Jacob Wen              2019-01-22  672  
f7faffa3 James Chapman          2010-04-02 @673                 u32 l2h = 
ntohl(*(__be32 *) ptr);
f7faffa3 James Chapman          2010-04-02  674  
f7faffa3 James Chapman          2010-04-02  675                 if (l2h & 
0x40000000) {
f7faffa3 James Chapman          2010-04-02  676                         ns = 
l2h & 0x00ffffff;
f7faffa3 James Chapman          2010-04-02  677  
f7faffa3 James Chapman          2010-04-02  678                         /* 
Store L2TP info in the skb */
f7faffa3 James Chapman          2010-04-02  679                         
L2TP_SKB_CB(skb)->ns = ns;
f7faffa3 James Chapman          2010-04-02  680                         
L2TP_SKB_CB(skb)->has_seq = 1;
f7faffa3 James Chapman          2010-04-02  681  
a4ca44fa Joe Perches            2012-05-16  682                         
l2tp_dbg(session, L2TP_MSG_SEQ,
f7faffa3 James Chapman          2010-04-02  683                                 
 "%s: recv data ns=%u, session nr=%u\n",
f7faffa3 James Chapman          2010-04-02  684                                 
 session->name, ns, session->nr);
f7faffa3 James Chapman          2010-04-02  685                 }
62e7b6a5 Lorenzo Bianconi       2018-01-16  686                 ptr += 4;
f7faffa3 James Chapman          2010-04-02  687         }
f7faffa3 James Chapman          2010-04-02  688  
fd558d18 James Chapman          2010-04-02  689         if 
(L2TP_SKB_CB(skb)->has_seq) {
fd558d18 James Chapman          2010-04-02  690                 /* Received a 
packet with sequence numbers. If we're the LNS,
fd558d18 James Chapman          2010-04-02  691                  * check if we 
sre sending sequence numbers and if not,
fd558d18 James Chapman          2010-04-02  692                  * configure it 
so.
fd558d18 James Chapman          2010-04-02  693                  */
fd558d18 James Chapman          2010-04-02  694                 if 
((!session->lns_mode) && (!session->send_seq)) {
a4ca44fa Joe Perches            2012-05-16  695                         
l2tp_info(session, L2TP_MSG_SEQ,
fd558d18 James Chapman          2010-04-02  696                                 
  "%s: requested to enable seq numbers by LNS\n",
fd558d18 James Chapman          2010-04-02  697                                 
  session->name);
3f9b9770 Asbjørn Sloth Tønnesen 2016-11-07  698                         
session->send_seq = 1;
f7faffa3 James Chapman          2010-04-02  699                         
l2tp_session_set_header_len(session, tunnel->version);
fd558d18 James Chapman          2010-04-02  700                 }
fd558d18 James Chapman          2010-04-02  701         } else {
fd558d18 James Chapman          2010-04-02  702                 /* No sequence 
numbers.
fd558d18 James Chapman          2010-04-02  703                  * If user has 
configured mandatory sequence numbers, discard.
fd558d18 James Chapman          2010-04-02  704                  */
fd558d18 James Chapman          2010-04-02  705                 if 
(session->recv_seq) {
a4ca44fa Joe Perches            2012-05-16  706                         
l2tp_warn(session, L2TP_MSG_SEQ,
a4ca44fa Joe Perches            2012-05-16  707                                 
  "%s: recv data has no seq numbers when required. Discarding.\n",
a4ca44fa Joe Perches            2012-05-16  708                                 
  session->name);
7b7c0719 Tom Parkin             2013-03-19  709                         
atomic_long_inc(&session->stats.rx_seq_discards);
fd558d18 James Chapman          2010-04-02  710                         goto 
discard;
fd558d18 James Chapman          2010-04-02  711                 }
fd558d18 James Chapman          2010-04-02  712  
fd558d18 James Chapman          2010-04-02  713                 /* If we're the 
LAC and we're sending sequence numbers, the
fd558d18 James Chapman          2010-04-02  714                  * LNS has 
requested that we no longer send sequence numbers.
fd558d18 James Chapman          2010-04-02  715                  * If we're the 
LNS and we're sending sequence numbers, the
fd558d18 James Chapman          2010-04-02  716                  * LAC is 
broken. Discard the frame.
fd558d18 James Chapman          2010-04-02  717                  */
fd558d18 James Chapman          2010-04-02  718                 if 
((!session->lns_mode) && (session->send_seq)) {
a4ca44fa Joe Perches            2012-05-16  719                         
l2tp_info(session, L2TP_MSG_SEQ,
fd558d18 James Chapman          2010-04-02  720                                 
  "%s: requested to disable seq numbers by LNS\n",
fd558d18 James Chapman          2010-04-02  721                                 
  session->name);
fd558d18 James Chapman          2010-04-02  722                         
session->send_seq = 0;
f7faffa3 James Chapman          2010-04-02  723                         
l2tp_session_set_header_len(session, tunnel->version);
fd558d18 James Chapman          2010-04-02  724                 } else if 
(session->send_seq) {
a4ca44fa Joe Perches            2012-05-16  725                         
l2tp_warn(session, L2TP_MSG_SEQ,
a4ca44fa Joe Perches            2012-05-16  726                                 
  "%s: recv data has no seq numbers when required. Discarding.\n",
a4ca44fa Joe Perches            2012-05-16  727                                 
  session->name);
7b7c0719 Tom Parkin             2013-03-19  728                         
atomic_long_inc(&session->stats.rx_seq_discards);
fd558d18 James Chapman          2010-04-02  729                         goto 
discard;
fd558d18 James Chapman          2010-04-02  730                 }
fd558d18 James Chapman          2010-04-02  731         }
fd558d18 James Chapman          2010-04-02  732  
900631ee James Chapman          2018-01-03  733         /* Session data offset 
is defined only for L2TPv2 and is
900631ee James Chapman          2018-01-03  734          * indicated by an 
optional 16-bit value in the header.
f7faffa3 James Chapman          2010-04-02  735          */
f7faffa3 James Chapman          2010-04-02  736         if (tunnel->version == 
L2TP_HDR_VER_2) {
fd558d18 James Chapman          2010-04-02  737                 /* If offset 
bit set, skip it. */
fd558d18 James Chapman          2010-04-02  738                 if (hdrflags & 
L2TP_HDRFLAG_O) {
31c773ec Jacob Wen              2019-01-22  739                         if 
(!pskb_may_pull(skb, ptr - optr + 2))
31c773ec Jacob Wen              2019-01-22  740                                 
goto discard;
31c773ec Jacob Wen              2019-01-22  741  
fd558d18 James Chapman          2010-04-02  742                         offset 
= ntohs(*(__be16 *)ptr);
fd558d18 James Chapman          2010-04-02  743                         ptr += 
2 + offset;
fd558d18 James Chapman          2010-04-02  744                 }
900631ee James Chapman          2018-01-03  745         }
fd558d18 James Chapman          2010-04-02  746  
fd558d18 James Chapman          2010-04-02  747         offset = ptr - optr;
fd558d18 James Chapman          2010-04-02  748         if (!pskb_may_pull(skb, 
offset))
fd558d18 James Chapman          2010-04-02  749                 goto discard;
fd558d18 James Chapman          2010-04-02  750  
fd558d18 James Chapman          2010-04-02  751         __skb_pull(skb, offset);
fd558d18 James Chapman          2010-04-02  752  
fd558d18 James Chapman          2010-04-02  753         /* Prepare skb for 
adding to the session's reorder_q.  Hold
fd558d18 James Chapman          2010-04-02  754          * packets for max 
reorder_timeout or 1 second if not
fd558d18 James Chapman          2010-04-02  755          * reordering.
fd558d18 James Chapman          2010-04-02  756          */
fd558d18 James Chapman          2010-04-02  757         
L2TP_SKB_CB(skb)->length = length;
fd558d18 James Chapman          2010-04-02  758         
L2TP_SKB_CB(skb)->expires = jiffies +
fd558d18 James Chapman          2010-04-02  759                 
(session->reorder_timeout ? session->reorder_timeout : HZ);
fd558d18 James Chapman          2010-04-02  760  
fd558d18 James Chapman          2010-04-02  761         /* Add packet to the 
session's receive queue. Reordering is done here, if
fd558d18 James Chapman          2010-04-02  762          * enabled. Saved L2TP 
protocol info is stored in skb->sb[].
fd558d18 James Chapman          2010-04-02  763          */
fd558d18 James Chapman          2010-04-02  764         if 
(L2TP_SKB_CB(skb)->has_seq) {
b6dc01a4 James Chapman          2013-07-02  765                 if 
(l2tp_recv_data_seq(session, skb))
fd558d18 James Chapman          2010-04-02  766                         goto 
discard;
fd558d18 James Chapman          2010-04-02  767         } else {
fd558d18 James Chapman          2010-04-02  768                 /* No sequence 
numbers. Add the skb to the tail of the
fd558d18 James Chapman          2010-04-02  769                  * reorder 
queue. This ensures that it will be
fd558d18 James Chapman          2010-04-02  770                  * delivered 
after all previous sequenced skbs.
fd558d18 James Chapman          2010-04-02  771                  */
fd558d18 James Chapman          2010-04-02  772                 
skb_queue_tail(&session->reorder_q, skb);
fd558d18 James Chapman          2010-04-02  773         }
fd558d18 James Chapman          2010-04-02  774  
fd558d18 James Chapman          2010-04-02  775         /* Try to dequeue as 
many skbs from reorder_q as we can. */
fd558d18 James Chapman          2010-04-02  776         
l2tp_recv_dequeue(session);
fd558d18 James Chapman          2010-04-02  777  
f7faffa3 James Chapman          2010-04-02  778         return;
fd558d18 James Chapman          2010-04-02  779  
fd558d18 James Chapman          2010-04-02  780  discard:
7b7c0719 Tom Parkin             2013-03-19  781         
atomic_long_inc(&session->stats.rx_errors);
fd558d18 James Chapman          2010-04-02  782         kfree_skb(skb);
f7faffa3 James Chapman          2010-04-02  783  }
f7faffa3 James Chapman          2010-04-02  784  
EXPORT_SYMBOL(l2tp_recv_common);
f7faffa3 James Chapman          2010-04-02  785  

:::::: The code at line 673 was first introduced by commit
:::::: f7faffa3ff8ef6ae712ef16312b8a2aa7a1c95fe l2tp: Add L2TPv3 protocol 
support

:::::: TO: James Chapman <[email protected]>
:::::: CC: David S. Miller <[email protected]>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Attachment: .config.gz
Description: application/gzip

Reply via email to