Am Mittwoch, 8. Juni 2016, 12:14:49 schrieb Mat Martineau: Hi Mat, > On Wed, 8 Jun 2016, Stephan Mueller wrote: > > Am Dienstag, 7. Juni 2016, 17:28:07 schrieb Mat Martineau: > > > > Hi Mat, > > > >>> + used = ctx->used; > >>> + > >>> + /* convert iovecs of output buffers into scatterlists */ > >>> + while (iov_iter_count(&msg->msg_iter)) { > >>> + /* make one iovec available as scatterlist */ > >>> + err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter, > >>> + iov_iter_count(&msg->msg_iter)); > >>> + if (err < 0) > >>> + goto unlock; > >>> + usedpages += err; > >>> + /* chain the new scatterlist with previous one */ > >>> + if (cnt) > >>> + af_alg_link_sg(&ctx->rsgl[cnt - 1], &ctx->rsgl[cnt]); > >>> + > >>> + iov_iter_advance(&msg->msg_iter, err); > >>> + cnt++; > >>> + } > >>> + > >>> + /* ensure output buffer is sufficiently large */ > >>> + if (usedpages < akcipher_calcsize(ctx)) { > >>> + err = -EMSGSIZE; > >>> + goto unlock; > >>> + } > >> > >> Why is the size of the output buffer enforced here instead of depending > >> on > >> the algorithm implementation? > > > > akcipher_calcsize calls crypto_akcipher_maxsize to get the maximum size > > the > > algorithm generates as output during its operation. > > > > The code ensures that the caller provided at least that amount of memory > > for the kernel to store its data in. This check therefore is present to > > ensure the kernel does not overstep memory boundaries in user space. > > Yes, it's understood that the userspace buffer length must not be > exceeded. But dst_len is part of the akcipher_request struct, so why does > it need to be checked *here* when it is also checked later? I am always uneasy when the kernel has a user space interface and expects layers deep down inside the kernel to check for user space related boundaries. Note, we do not hand the __user flag down, so sparse and other tools cannot detect whether a particular cipher implementation has the right checks. I therefore always would like to check parameters at the interface handling logic. Cryptographers rightly should worry about their code implementing the cipher correctly. But I do not think that the cipher implementations should worry about security implications since they may be called from user space. > > > What is your concern? > > Userspace must allocate larger buffers than it knows are necessary for > expected results. > > It looks like the software rsa implementation handles shorter output > buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is > too small), however I see at least one hardware rsa driver that requires > the output buffer to be the maximum size. But this inconsistency might be > best addressed within the software cipher or drivers rather than in > recvmsg. Is your concern that we have a double check check for lengths here? If yes, I think we can live with an additional if() here. Or is your concern that the user space interface restricts things too much and thus prevents a valid use case? Ciao Stephan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html