On Fri, 31 Aug 2018 09:53:46 +0200 (CEST)
Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> wrote:
> Hi Stefano,
>
> On Thu, 30 Aug 2018, Stefano Brivio wrote:
>
> > > > @@ -2014,7 +2021,11 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
> > > > if (session->lineno != 0 &&
> > > > (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)) {
> > > > /* Save setname for the next possible aggregated restore line */
> > > > - strcpy(session->saved_setname, ipset_data_setname(data));
> > > > + const char *setname = ipset_data_setname(data);
> > > > + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> > > > + return ipset_err(session,
> > > > + "Invalid command: setname missing or too long");
> > > > + strcpy(session->saved_setname, setname);
> > >
> > > I don't understand this part: it is from userspace to kernel and ADD/DEL
> > > commands already verified that setname is filled out, in build_msg().
> >
> > Right, I don't need to check that setname is there. Still, its length is
> > not checked as far as I can see. Would you suggest that I move the
> > length check to build_msg()?
>
> build_msg() gets all the data for the netlink message from the ipset_data
> structure. The structure is filled up by parsers and the parsers verify
> the input, in this case ipset_parse_setname(). So the checking is
> unnecessary here.
Thanks, I see now, that was already called by parse_commandline(). I'll
drop this part then.
--
Stefano
