On Fri, May 31, 2019 at 10:45 PM Gen Zhang <blackgod016574@xxxxxxxxx> wrote: > On Sat, Jun 01, 2019 at 03:25:27AM +0100, Al Viro wrote: > > On Sat, Jun 01, 2019 at 10:15:26AM +0800, Gen Zhang wrote: > > > In selinux_sb_eat_lsm_opts(), 'arg' is allocated by kmemdup_nul(). It > > > returns NULL when fails. So 'arg' should be checked. And 'mnt_opts' > > > should be freed when error. > > > > What's the latter one for? On failure we'll get to put_fs_context() > > pretty soon, so > > security_free_mnt_opts(&fc->security); > > will be called just fine. Leaving it allocated on failure is fine... > Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > >It seems like we should also check for, and potentially free *mnt_opts > >as the selinux_add_opt() error handling does just below this change, > >yes? If that is the case we might want to move that error handling > >code to the bottom of the function and jump there on error. > I am not familiar with this part. So could you please show the function > call sequence? I'm not sure I understand your question above, but I did review your latest patch and agree with Ondrej's comment regarding the ret/rc variable. If you make that change I think we can merge this into selinux/stable-5.2. -- paul moore www.paul-moore.com
