On 03/29/2018 11:48 AM, Yuli Khodorkovskiy wrote: > > > On Thu, Mar 29, 2018 at 9:49 AM, Stephen Smalley <[email protected] > <mailto:[email protected]>> wrote: > > On 03/28/2018 11:40 PM, Yuli Khodorkovskiy wrote: > > Keep track of line numbers for each file context in > > selabel_handle. If an error occurs in selabel_fini(), the > > line number of an invalid file context is echoed to the user. > > > > Signed-off-by: Yuli Khodorkovskiy <[email protected] > <mailto:[email protected]>> > > --- > > libselinux/src/label.c | 2 +- > > libselinux/src/label_file.h | 1 + > > libselinux/src/label_internal.h | 1 + > > 3 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libselinux/src/label.c b/libselinux/src/label.c > > index e642a97b..d9a58ce9 100644 > > --- a/libselinux/src/label.c > > +++ b/libselinux/src/label.c > > @@ -143,7 +143,7 @@ static int selabel_fini(struct selabel_handle *rec, > > struct selabel_lookup_rec *lr, > > int translating) > > { > > - if (compat_validate(rec, lr, rec->spec_file, 0)) > > + if (compat_validate(rec, lr, rec->spec_file, lr->lineno)) > > return -1; > > > > if (translating && !lr->ctx_trans && > > diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h > > index aa576d8e..4780ae48 100644 > > --- a/libselinux/src/label_file.h > > +++ b/libselinux/src/label_file.h > > @@ -472,6 +472,7 @@ static inline int process_line(struct > selabel_handle *rec, > > spec_arr[nspec].mode = 0; > > > > spec_arr[nspec].lr.ctx_raw = context; > > + spec_arr[nspec].lr.lineno = lineno; > > > > /* > > * bump data->nspecs to cause closef() to cover it in its free > > diff --git a/libselinux/src/label_internal.h > b/libselinux/src/label_internal.h > > index c55efb75..0e020557 100644 > > --- a/libselinux/src/label_internal.h > > +++ b/libselinux/src/label_internal.h > > @@ -73,6 +73,7 @@ struct selabel_lookup_rec { > > char * ctx_raw; > > char * ctx_trans; > > int validated; > > + unsigned lineno; > > }; > > > > struct selabel_handle { > > > > I think this is ok, but wanted to double check: does this work correctly > when file contexts are loaded from > file_contexts.bin instead? It looks to me as if the lineno will be left > as 0 in that case and the > code will handle that correctly. > > > Compiling a file_contexts.bin with sefcontext_compile will give you the line > number: > > sefcontext_compile /etc/selinux/targeted/contexts/files/file_contexts -o > file_contexts.bin -p /etc/selinux/targeted/policy/policy.31 > libsepol.sepol_context_from_string: malformed context "test" > libsepol.sepol_context_from_string: could not construct context from string > libsepol.context_from_string: could not create context structure > libsepol.sepol_context_to_sid: could not convert test to sid > /etc/selinux/targeted/contexts/files/file_contexts: line 6132 has invalid > context test > sefcontext_compile: process_file failed > > Using file_contexts.bin for relabeling that I generated with no validation > will not report a line number: > > restorecon: /etc/selinux/targeted/contexts/files/file_contexts: has invalid > context test > > I'll see if I can associate the line number with each regex in > sefcontext_compile.
That's fine if you want to do it but not necessary for these patches to be merged; I just wanted to ensure that we don't have garbage output as a result of these patches. I'd do it as a separate patch regardless since it likely requires a new binary format version for the file_contexts.bin files. > The other question is whether we correctly report the file name when the > entry comes from a file other > than file_contexts itself, e.g. file_contexts.local, .homedirs, ... Not > your bug if we don't but wondered. > > > It is reporting the line number correctly, but the filename is incorrect. > I'll update this. Again, not strictly necessary for these patches since you didn't introduce the bug, and probably ought to be its own separate patch.
