Re: [PATCH net] sctp: frag_point sanity check

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

 



Hi Jakub,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Jakub-Audykowicz/sctp-frag_point-sanity-check/20181206-011917
config: x86_64-randconfig-x015-12051035 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
                    from net/sctp/chunk.c:36:
   net/sctp/chunk.c: In function 'sctp_datamsg_from_user':
   include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t {aka long unsigned int}' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:429:10: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
             ^~~
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
    #define KERN_WARNING KERN_SOH "4" /* warning conditions */
                         ^~~~~~~~
   include/linux/printk.h:445:21: note: in expansion of macro 'KERN_WARNING'
     printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
                        ^~~~~~~~~~~~
>> net/sctp/chunk.c:197:3: note: in expansion of macro 'pr_warn_ratelimited'
      pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
      ^~~~~~~~~~~~~~~~~~~

vim +/pr_warn_ratelimited +197 net/sctp/chunk.c

   156	
   157	
   158	/* A data chunk can have a maximum payload of (2^16 - 20).  Break
   159	 * down any such message into smaller chunks.  Opportunistically, fragment
   160	 * the chunks down to the current MTU constraints.  We may get refragmented
   161	 * later if the PMTU changes, but it is _much better_ to fragment immediately
   162	 * with a reasonable guess than always doing our fragmentation on the
   163	 * soft-interrupt.
   164	 */
   165	struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
   166						    struct sctp_sndrcvinfo *sinfo,
   167						    struct iov_iter *from)
   168	{
   169		size_t len, first_len, max_data, remaining;
   170		size_t msg_len = iov_iter_count(from);
   171		struct sctp_shared_key *shkey = NULL;
   172		struct list_head *pos, *temp;
   173		struct sctp_chunk *chunk;
   174		struct sctp_datamsg *msg;
   175		int err;
   176	
   177		msg = sctp_datamsg_new(GFP_KERNEL);
   178		if (!msg)
   179			return ERR_PTR(-ENOMEM);
   180	
   181		/* Note: Calculate this outside of the loop, so that all fragments
   182		 * have the same expiration.
   183		 */
   184		if (asoc->peer.prsctp_capable && sinfo->sinfo_timetolive &&
   185		    (SCTP_PR_TTL_ENABLED(sinfo->sinfo_flags) ||
   186		     !SCTP_PR_POLICY(sinfo->sinfo_flags)))
   187			msg->expires_at = jiffies +
   188					  msecs_to_jiffies(sinfo->sinfo_timetolive);
   189	
   190		/* This is the biggest possible DATA chunk that can fit into
   191		 * the packet
   192		 */
   193		max_data = asoc->frag_point;
   194		if (unlikely(!max_data)) {
   195			max_data = sctp_min_frag_point(sctp_sk(asoc->base.sk),
   196						       sctp_datachk_len(&asoc->stream));
 > 197			pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum (%d)",
   198					    __func__, asoc, max_data);
   199		}
   200	
   201		/* If the the peer requested that we authenticate DATA chunks
   202		 * we need to account for bundling of the AUTH chunks along with
   203		 * DATA.
   204		 */
   205		if (sctp_auth_send_cid(SCTP_CID_DATA, asoc)) {
   206			struct sctp_hmac *hmac_desc = sctp_auth_asoc_get_hmac(asoc);
   207	
   208			if (hmac_desc)
   209				max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
   210						      hmac_desc->hmac_len);
   211	
   212			if (sinfo->sinfo_tsn &&
   213			    sinfo->sinfo_ssn != asoc->active_key_id) {
   214				shkey = sctp_auth_get_shkey(asoc, sinfo->sinfo_ssn);
   215				if (!shkey) {
   216					err = -EINVAL;
   217					goto errout;
   218				}
   219			} else {
   220				shkey = asoc->shkey;
   221			}
   222		}
   223	
   224		/* Set first_len and then account for possible bundles on first frag */
   225		first_len = max_data;
   226	
   227		/* Check to see if we have a pending SACK and try to let it be bundled
   228		 * with this message.  Do this if we don't have any data queued already.
   229		 * To check that, look at out_qlen and retransmit list.
   230		 * NOTE: we will not reduce to account for SACK, if the message would
   231		 * not have been fragmented.
   232		 */
   233		if (timer_pending(&asoc->timers[SCTP_EVENT_TIMEOUT_SACK]) &&
   234		    asoc->outqueue.out_qlen == 0 &&
   235		    list_empty(&asoc->outqueue.retransmit) &&
   236		    msg_len > max_data)
   237			first_len -= SCTP_PAD4(sizeof(struct sctp_sack_chunk));
   238	
   239		/* Encourage Cookie-ECHO bundling. */
   240		if (asoc->state < SCTP_STATE_COOKIE_ECHOED)
   241			first_len -= SCTP_ARBITRARY_COOKIE_ECHO_LEN;
   242	
   243		/* Account for a different sized first fragment */
   244		if (msg_len >= first_len) {
   245			msg->can_delay = 0;
   246			if (msg_len > first_len)
   247				SCTP_INC_STATS(sock_net(asoc->base.sk),
   248					       SCTP_MIB_FRAGUSRMSGS);
   249		} else {
   250			/* Which may be the only one... */
   251			first_len = msg_len;
   252		}
   253	
   254		/* Create chunks for all DATA chunks. */
   255		for (remaining = msg_len; remaining; remaining -= len) {
   256			u8 frag = SCTP_DATA_MIDDLE_FRAG;
   257	
   258			if (remaining == msg_len) {
   259				/* First frag, which may also be the last */
   260				frag |= SCTP_DATA_FIRST_FRAG;
   261				len = first_len;
   262			} else {
   263				/* Middle frags */
   264				len = max_data;
   265			}
   266	
   267			if (len >= remaining) {
   268				/* Last frag, which may also be the first */
   269				len = remaining;
   270				frag |= SCTP_DATA_LAST_FRAG;
   271	
   272				/* The application requests to set the I-bit of the
   273				 * last DATA chunk of a user message when providing
   274				 * the user message to the SCTP implementation.
   275				 */
   276				if ((sinfo->sinfo_flags & SCTP_EOF) ||
   277				    (sinfo->sinfo_flags & SCTP_SACK_IMMEDIATELY))
   278					frag |= SCTP_DATA_SACK_IMM;
   279			}
   280	
   281			chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
   282							       GFP_KERNEL);
   283			if (!chunk) {
   284				err = -ENOMEM;
   285				goto errout;
   286			}
   287	
   288			err = sctp_user_addto_chunk(chunk, len, from);
   289			if (err < 0)
   290				goto errout_chunk_free;
   291	
   292			chunk->shkey = shkey;
   293	
   294			/* Put the chunk->skb back into the form expected by send.  */
   295			__skb_pull(chunk->skb, (__u8 *)chunk->chunk_hdr -
   296					       chunk->skb->data);
   297	
   298			sctp_datamsg_assign(msg, chunk);
   299			list_add_tail(&chunk->frag_list, &msg->chunks);
   300		}
   301	
   302		return msg;
   303	
   304	errout_chunk_free:
   305		sctp_chunk_free(chunk);
   306	
   307	errout:
   308		list_for_each_safe(pos, temp, &msg->chunks) {
   309			list_del_init(pos);
   310			chunk = list_entry(pos, struct sctp_chunk, frag_list);
   311			sctp_chunk_free(chunk);
   312		}
   313		sctp_datamsg_put(msg);
   314	
   315		return ERR_PTR(err);
   316	}
   317	

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

Attachment: .config.gz
Description: application/gzip


[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux